From 0f554506a3fc6a21e714adf38cc3eb0ffe1bb452 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 14 Oct 2026 15:31:07 +0200 Subject: [PATCH 1/3] driver core: fix parent lock use-after-free on USB disconnect USB interfaces should be probed with their parent USB device lock held and the USB bus therefore sets the need_parent_lock bus flag so that driver core takes the parent lock when needed. Driver core is only guaranteed to hold a reference to the parent device during initial probe however and nothing prevents the device and its parent from being deregistered and the parent potentially freed, for example, while iterating over devices in driver_attach() or bus_rescan_devices_helper(). Since commit 524134d42231 ("USB: don't cancel queued resets when unbinding drivers") USB interfaces hold a reference to their parent USB device, but a parent hub may still go away while probing a USB device (e.g. via the drivers_probe sysfs interface). This can lead to a use-after-free when taking the parent device lock before checking the device->p->dead flag of the device being probed. As only USB interfaces actually need the parent lock, make the need_parent_lock feature even more fine grained by making it a device type rather than bus feature. By requiring that any users also make sure that their devices hold a reference to their parents (like USB does), driver core can lock a device and its parent without first checking if the device has been deregistered. Note that this will also increase parallelism when using asynchronous probe as USB devices connected to the same hub can now be probed in parallel. Fixes: bf74ad5bc417 ("[PATCH] Hold the device's parent's lock during probe and remove") Cc: stable@vger.kernel.org # 2.6.16: 524134d42231: USB: don't cancel queued resets when unbinding drivers Cc: stable@vger.kernel.org # 2.6.16 Cc: Alan Stern Signed-off-by: Johan Hovold Link: https://patch.msgid.link/20261014133109.1392265-2-johan@kernel.org [ Fix mention of bus vs. device type in __device_driver_{un}lock(). - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/base/bus.c | 4 ++-- drivers/base/dd.c | 8 ++++---- drivers/usb/core/driver.c | 1 - drivers/usb/core/message.c | 1 + include/linux/device.h | 5 +++++ include/linux/device/bus.h | 3 --- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index d17bd91490ee..d6668fb8ddea 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -849,10 +849,10 @@ static int __must_check bus_rescan_devices_helper(struct device *dev, int ret = 0; if (!dev->driver) { - if (dev->parent && dev->bus->need_parent_lock) + if (dev->type && dev->type->need_parent_lock) device_lock(dev->parent); ret = device_attach(dev); - if (dev->parent && dev->bus->need_parent_lock) + if (dev->type && dev->type->need_parent_lock) device_unlock(dev->parent); } return ret < 0 ? ret : 0; diff --git a/drivers/base/dd.c b/drivers/base/dd.c index f6525a7ee8c5..d7a82c436c45 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -1158,7 +1158,7 @@ void device_initial_probe(struct device *dev) /* * __device_driver_lock - acquire locks needed to manipulate dev->drv * @dev: Device we will update driver info for - * @parent: Parent device. Needed if the bus requires parent lock + * @parent: Parent device. Needed if the device type requires parent lock * * This function will take the required locks for manipulating dev->drv. * Normally this will just be the @dev lock, but when called for a USB @@ -1166,7 +1166,7 @@ void device_initial_probe(struct device *dev) */ static void __device_driver_lock(struct device *dev, struct device *parent) { - if (parent && dev->bus->need_parent_lock) + if (dev->type && dev->type->need_parent_lock && parent) device_lock(parent); device_lock(dev); } @@ -1174,7 +1174,7 @@ static void __device_driver_lock(struct device *dev, struct device *parent) /* * __device_driver_unlock - release locks needed to manipulate dev->drv * @dev: Device we will update driver info for - * @parent: Parent device. Needed if the bus requires parent lock + * @parent: Parent device. Needed if the device type requires parent lock * * This function will release the required locks for manipulating dev->drv. * Normally this will just be the @dev lock, but when called for a @@ -1183,7 +1183,7 @@ static void __device_driver_lock(struct device *dev, struct device *parent) static void __device_driver_unlock(struct device *dev, struct device *parent) { device_unlock(dev); - if (parent && dev->bus->need_parent_lock) + if (dev->type && dev->type->need_parent_lock && parent) device_unlock(parent); } diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 7f33fe5ba03b..bf660b15a0f6 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -2081,5 +2081,4 @@ const struct bus_type usb_bus_type = { .name = "usb", .match = usb_device_match, .uevent = usb_uevent, - .need_parent_lock = true, }; diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 75e2bfd744a9..e8a582fef09f 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1916,6 +1916,7 @@ const struct device_type usb_if_device_type = { .name = "usb_interface", .release = usb_release_interface, .uevent = usb_if_uevent, + .need_parent_lock = true, }; static struct usb_interface_assoc_descriptor *find_iad(struct usb_device *dev, diff --git a/include/linux/device.h b/include/linux/device.h index aee79fd6b32b..ae05205742c0 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -83,6 +83,10 @@ int subsys_virtual_register(const struct bus_type *subsys, * information, equivalent to the kobj_type of a kobject. * If "name" is specified, the uevent will contain it in * the DEVTYPE variable. + * + * @need_parent_lock: When probing or removing a device of this type, + * driver core should lock the device's parent. Note that + * the device must hold a reference to its parent. */ struct device_type { const char *name; @@ -93,6 +97,7 @@ struct device_type { void (*release)(struct device *dev); const struct dev_pm_ops *pm; + bool need_parent_lock; }; /** diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h index a38f7229b8f4..526407a371e2 100644 --- a/include/linux/device/bus.h +++ b/include/linux/device/bus.h @@ -68,8 +68,6 @@ struct fwnode_handle; * @driver_override: Set to true if this bus supports the driver_override * mechanism, which allows userspace to force a specific * driver to bind to a device via a sysfs attribute. - * @need_parent_lock: When probing or removing a device on this bus, the - * device core should lock the device's parent. * * A bus is a channel between the processor and one or more devices. For the * purposes of the device model, all devices are connected via a bus, even if @@ -110,7 +108,6 @@ struct bus_type { const struct dev_pm_ops *pm; bool driver_override; - bool need_parent_lock; }; int __must_check bus_register(const struct bus_type *bus); From 4059f8986c8f50102a04335cc1da4921dfd262d8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 14 Oct 2026 15:31:08 +0200 Subject: [PATCH 2/3] driver core: take parent lock on deferred probe USB interfaces should be probed with the parent USB device lock held, but this is not honoured when probe is deferred. Add a new bus_probe_device_deferred() helper to handle deferred probe where the parent device lock is not held. Fixes: d1c3414c2a9d ("drivercore: Add driver probe deferral mechanism") Reported-by: Mario Peter Link: https://lore.kernel.org/20261007131514.1260696-1-mario.peter@leica-geosystems.com Cc: stable@vger.kernel.org # 3.4 Signed-off-by: Johan Hovold Link: https://patch.msgid.link/20261014133109.1392265-3-johan@kernel.org Signed-off-by: Danilo Krummrich --- drivers/base/base.h | 1 + drivers/base/bus.c | 28 +++++++++++++++++++++------- drivers/base/dd.c | 7 ++++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index a5b7abc10ff0..3c36a2128f88 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -154,6 +154,7 @@ struct kobject *virtual_device_parent(void); int bus_add_device(struct device *dev); void bus_probe_device(struct device *dev); +void bus_probe_device_deferred(struct device *dev); void bus_remove_device(struct device *dev); void bus_notify(struct device *dev, enum bus_notifier_event value); bool bus_is_registered(const struct bus_type *bus); diff --git a/drivers/base/bus.c b/drivers/base/bus.c index d6668fb8ddea..43d131fd8085 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -603,13 +603,7 @@ int bus_add_device(struct device *dev) return error; } -/** - * bus_probe_device - probe drivers for a new device - * @dev: device to probe - * - * - Automatically probe for a driver if the bus allows it. - */ -void bus_probe_device(struct device *dev) +static void __bus_probe_device(struct device *dev) { struct subsys_private *sp = bus_to_subsys(dev->bus); struct subsys_interface *sif; @@ -627,6 +621,26 @@ void bus_probe_device(struct device *dev) subsys_put(sp); } +/** + * bus_probe_device - probe drivers for a new device + * @dev: device to probe + * + * - Automatically probe for a driver if the bus allows it. + */ +void bus_probe_device(struct device *dev) +{ + __bus_probe_device(dev); +} + +void bus_probe_device_deferred(struct device *dev) +{ + if (dev->type && dev->type->need_parent_lock) + device_lock(dev->parent); + __bus_probe_device(dev); + if (dev->type && dev->type->need_parent_lock) + device_unlock(dev->parent); +} + /** * bus_remove_device - remove device from bus * @dev: device to be removed diff --git a/drivers/base/dd.c b/drivers/base/dd.c index d7a82c436c45..04e9977ed743 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -86,8 +86,9 @@ static void deferred_probe_work_func(struct work_struct *work) /* * This block processes every device in the deferred 'active' list. * Each device is removed from the active list and passed to - * bus_probe_device() to re-attempt the probe. The loop continues - * until every device in the active list is removed and retried. + * bus_probe_device_deferred() to re-attempt the probe. The loop + * continues until every device in the active list is removed and + * retried. * * Note: Once the device is removed from the list and the mutex is * released, it is possible for the device get freed by another thread @@ -121,7 +122,7 @@ static void deferred_probe_work_func(struct work_struct *work) device_pm_move_to_tail(dev); dev_dbg(dev, "Retrying from deferred list\n"); - bus_probe_device(dev); + bus_probe_device_deferred(dev); mutex_lock(&deferred_probe_mutex); put_device(dev); From 187a62f99da29b1a55602b0d401a783d8dc86161 Mon Sep 17 00:00:00 2001 From: Mario Peter Date: Wed, 14 Oct 2026 15:31:09 +0200 Subject: [PATCH 3/3] driver core: take parent lock in async device attach For device types with need_parent_lock set (only USB interfaces), probe() must run with the parent device locked. A synchronous attach after device_add() gets this from the caller, e.g. usb_set_configuration() holds the udev lock while adding interfaces. __device_attach_async_helper() runs the probe from an async worker instead, where that lock isn't held, and only takes device_lock(dev). With async probing enabled for the hub driver (e.g. driver_async_probe=* or driver_async_probe=hub), this races when a multi-TT hub is configured. For multi-TT hubs, hub_probe() calls usb_set_interface(hdev, 0, 1) to select the TT-per-port altsetting, and usb_set_interface() creates the endpoint devices of the interface a second time: usb_set_configuration() async worker [udev locked] device_add(intf) queues the probe ----------> __device_attach_async_helper() [intf locked, udev not locked] hub_probe() usb_set_interface(hdev, 0, 1) create_intf_ep_devs(intf) create_intf_ep_devs(intf) create_intf_ep_devs() checks and sets intf->ep_devs_created without a lock of its own. If usb_set_configuration() has created ep_81 but not yet set the flag, usb_set_interface() skips removing the endpoint devices of the old altsetting and creates ep_81 again. With a synchronous probe, hub_probe() runs inside device_add() with the udev lock held, and usb_set_configuration() then finds ep_devs_created already set. Single-TT hubs don't call usb_set_interface() from hub_probe() and don't show this warning, but their probe runs without the udev lock as well. On an i.MX8MM board with an onboard USB2514 hub this hit 11 of 100 boots: sysfs: cannot create duplicate filename '.../1-1/1-1:1.0/ep_81' Call trace: sysfs_warn_dup usb_create_ep_devs create_intf_ep_devs usb_set_interface hub_probe usb_probe_interface really_probe __device_attach_async_helper async_run_entry_fn It also shows on v7.3-rc6 in QEMU, with dummy_hcd and a raw-gadget program emulating a multi-TT hub: re-authorizing the hub in a loop, while a SCHED_FIFO task preempts the writing task at random points for 3 ms, hit the warning in 29 of 6000 iterations. Take the parent lock in __device_attach_async_helper() as well, like __driver_attach_async_helper() does, and move __device_driver_lock/unlock() up for that. With this the warning was gone in 300 boots on the board and in 6000 iterations in QEMU. Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers") Signed-off-by: Mario Peter Cc: stable@vger.kernel.org # 4.2 Cc: Dmitry Torokhov [ johan: switch to async_schedule_dev_nocall() to avoid deadlock if async_schedule_dev() runs synchronously when parent lock is held ] Signed-off-by: Johan Hovold Link: https://patch.msgid.link/20261014133109.1392265-4-johan@kernel.org [ Fold the nested conditions into a single if statement; correct commit message to mention "device types" rather than "buses" for "need_parent_lock". - Danilo ] Signed-off-by: Danilo Krummrich --- drivers/base/dd.c | 83 +++++++++++++++++++++++++---------------------- 1 file changed, 45 insertions(+), 38 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 04e9977ed743..52123251965d 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -1030,6 +1030,38 @@ static int __device_attach_driver(struct device_driver *drv, void *_data) return ret == 0; } +/* + * __device_driver_lock - acquire locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the device type requires parent lock + * + * This function will take the required locks for manipulating dev->drv. + * Normally this will just be the @dev lock, but when called for a USB + * interface, @parent lock will be held as well. + */ +static void __device_driver_lock(struct device *dev, struct device *parent) +{ + if (dev->type && dev->type->need_parent_lock && parent) + device_lock(parent); + device_lock(dev); +} + +/* + * __device_driver_unlock - release locks needed to manipulate dev->drv + * @dev: Device we will update driver info for + * @parent: Parent device. Needed if the device type requires parent lock + * + * This function will release the required locks for manipulating dev->drv. + * Normally this will just be the @dev lock, but when called for a + * USB interface, @parent lock will be released as well. + */ +static void __device_driver_unlock(struct device *dev, struct device *parent) +{ + device_unlock(dev); + if (dev->type && dev->type->need_parent_lock && parent) + device_unlock(parent); +} + static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) { struct device *dev = _dev; @@ -1039,7 +1071,7 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) .want_async = true, }; - device_lock(dev); + __device_driver_lock(dev, dev->parent); /* * Check if device has already been removed or claimed. This may @@ -1061,7 +1093,7 @@ static void __device_attach_async_helper(void *_dev, async_cookie_t cookie) if (dev->parent) pm_runtime_put(dev->parent); out_unlock: - device_unlock(dev); + __device_driver_unlock(dev, dev->parent); put_device(dev); } @@ -1118,8 +1150,11 @@ static int __device_attach(struct device *dev, bool allow_async) } out_unlock: device_unlock(dev); - if (async) - async_schedule_dev(__device_attach_async_helper, dev); + if (async && !async_schedule_dev_nocall(__device_attach_async_helper, dev)) { + put_device(dev); + ret = 2; /* retry synchronously */ + } + return ret; } @@ -1146,48 +1181,20 @@ EXPORT_SYMBOL_GPL(device_attach); void device_initial_probe(struct device *dev) { struct subsys_private *sp = bus_to_subsys(dev->bus); + int ret; if (!sp) return; - if (sp->drivers_autoprobe) - __device_attach(dev, true); + if (sp->drivers_autoprobe) { + ret = __device_attach(dev, true); + if (ret == 2) + __device_attach(dev, false); + } subsys_put(sp); } -/* - * __device_driver_lock - acquire locks needed to manipulate dev->drv - * @dev: Device we will update driver info for - * @parent: Parent device. Needed if the device type requires parent lock - * - * This function will take the required locks for manipulating dev->drv. - * Normally this will just be the @dev lock, but when called for a USB - * interface, @parent lock will be held as well. - */ -static void __device_driver_lock(struct device *dev, struct device *parent) -{ - if (dev->type && dev->type->need_parent_lock && parent) - device_lock(parent); - device_lock(dev); -} - -/* - * __device_driver_unlock - release locks needed to manipulate dev->drv - * @dev: Device we will update driver info for - * @parent: Parent device. Needed if the device type requires parent lock - * - * This function will release the required locks for manipulating dev->drv. - * Normally this will just be the @dev lock, but when called for a - * USB interface, @parent lock will be released as well. - */ -static void __device_driver_unlock(struct device *dev, struct device *parent) -{ - device_unlock(dev); - if (dev->type && dev->type->need_parent_lock && parent) - device_unlock(parent); -} - /** * device_driver_attach - attach a specific driver to a specific device * @drv: Driver to attach