Race condition in device probing / driver binding

diff --git a/drivers/base/core.c b/drivers/base/core.c index 673d709fa7e645..bd2ddf2aab505f 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -3688,6 +3688,21 @@ int device_add(struct device *dev) fw_devlink_link_device(dev); } + /* + * The moment the device was linked into the bus's "klist_devices" in + * bus_add_device() then it's possible that probe could have been + * attempted in a different thread via userspace loading a driver + * matching the device. "ready_to_probe" being unset would have + * blocked those attempts. Now that all of the above initialization has + * happened, unblock probe. If probe happens through another thread + * after this point but before bus_probe_device() runs then it's fine. + * bus_probe_device() -> device_initial_probe() -> __device_attach() + * will notice (under device_lock) that the device is already bound. + */ + device_lock(dev); + dev_set_ready_to_probe(dev); + device_unlock(dev); + bus_probe_device(dev); /* diff --git a/drivers/base/dd.c b/drivers/base/dd.c index cb5046f0634de9..1dc1e3528043c3 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -836,6 +836,26 @@ static int __driver_probe_device(const struct device_driver *drv, struct device if (dev->driver) return -EBUSY; + /* + * In device_add(), the "struct device" gets linked into the subsystem's + * list of devices and broadcast to userspace (via uevent) before we're + * quite ready to probe. Those open pathways to driver probe before + * we've finished enough of device_add() to reliably support probe. + * Detect this and tell other pathways to try again later. device_add() + * itself will also try to probe immediately after setting + * "ready_to_probe". + */ + if (!dev_ready_to_probe(dev)) + return dev_err_probe(dev, -EPROBE_DEFER, "Device not ready to probe\n"); + + /* + * Set can_match = true after calling dev_ready_to_probe(), so + * driver_deferred_probe_add() won't actually add the device to the + * deferred probe list when dev_ready_to_probe() returns false. + * + * When dev_ready_to_probe() returns false, it means that device_add() + * will do another probe() attempt for us. + */ dev->can_match = true; dev_dbg(dev, "bus: '%s': %s: matched device with driver %s\n", drv->bus->name, __func__, drv->name); diff --git a/drivers/base/property.c b/drivers/base/property.c index d16e9c5f1921a1..8e0148a37fffb4 100644 --- a/drivers/base/property.c +++ b/drivers/base/property.c @@ -602,6 +602,8 @@ EXPORT_SYMBOL_GPL(fwnode_property_match_property_string); * %-ENOENT when the index is out of bounds, the index has an empty * reference or the property was not found * %-EINVAL on parse error + * %-ENOTCONN when the remote firmware node exists but has not been + * registered yet */ int fwnode_property_get_reference_args(const struct fwnode_handle *fwnode, const char *prop, const char *nargs_prop, diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c index a80575bf598b50..a19f8f722bc8e1 100644 --- a/drivers/base/swnode.c +++ b/drivers/base/swnode.c @@ -554,7 +554,7 @@ software_node_get_reference_args(const struct fwnode_handle *fwnode, return -EINVAL; if (!refnode) - return -ENOENT; + return -ENOTCONN; if (nargs_prop) { error = fwnode_property_read_u32(refnode, nargs_prop, &nargs_prop_val); diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c index b3edae0578c073..182e54e575ee9c 100644 --- a/fs/sysfs/group.c +++ b/fs/sysfs/group.c @@ -517,8 +517,11 @@ static int sysfs_group_attrs_change_owner(struct kobject *kobj, struct attribute *const *attr; for (i = 0, attr = grp->attrs; *attr; i++, attr++) { - if (grp->is_visible) { - mode = grp->is_visible(kobj, *attr, i); + if (grp->is_visible || grp->is_visible_const) { + if (grp->is_visible) + mode = grp->is_visible(kobj, *attr, i); + else + mode = grp->is_visible_const(kobj, *attr, i); if (mode & SYSFS_GROUP_INVISIBLE) break; if (!mode) diff --git a/include/linux/device.h b/include/linux/device.h index 67cec9ec0cd0c3..9c8fde6a3d866b 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -504,6 +504,22 @@ struct device_physical_location { bool lid; }; +/** + * enum struct_device_flags - Flags in struct device + * + * Each flag should have a set of accessor functions created via + * __create_dev_flag_accessors() for each access. + * + * @DEV_FLAG_READY_TO_PROBE: If set then device_add() has finished enough + * initialization that probe could be called. + * @DEV_FLAG_COUNT: Number of defined struct_device_flags. + */ +enum struct_device_flags { + DEV_FLAG_READY_TO_PROBE = 0, + + DEV_FLAG_COUNT +}; + /** * struct device - The basic device structure * @parent: The device's "parent" device, the device to which it is attached. @@ -599,6 +615,7 @@ struct device_physical_location { * @dma_skip_sync: DMA sync operations can be skipped for coherent buffers. * @dma_iommu: Device is using default IOMMU implementation for DMA and * doesn't rely on dma_ops structure. + * @flags: DEV_FLAG_XXX flags. Use atomic bitfield operations to modify. * * At the lowest level, every device in a Linux system is represented by an * instance of struct device. The device structure contains the information @@ -721,8 +738,36 @@ struct device { #ifdef CONFIG_IOMMU_DMA bool dma_iommu:1; #endif + + DECLARE_BITMAP(flags, DEV_FLAG_COUNT); }; +#define __create_dev_flag_accessors(accessor_name, flag_name) \ +static inline bool dev_##accessor_name(const struct device *dev) \ +{ \ + return test_bit(flag_name, dev->flags); \ +} \ +static inline void dev_set_##accessor_name(struct device *dev) \ +{ \ + set_bit(flag_name, dev->flags); \ +} \ +static inline void dev_clear_##accessor_name(struct device *dev) \ +{ \ + clear_bit(flag_name, dev->flags); \ +} \ +static inline void dev_assign_##accessor_name(struct device *dev, bool value) \ +{ \ + assign_bit(flag_name, dev->flags, value); \ +} \ +static inline bool dev_test_and_set_##accessor_name(struct device *dev) \ +{ \ + return test_and_set_bit(flag_name, dev->flags); \ +} + +__create_dev_flag_accessors(ready_to_probe, DEV_FLAG_READY_TO_PROBE); + +#undef __create_dev_flag_accessors + /** * struct device_link - Device link representation. * @supplier: The device on the supplier end of the link.