[PATCH v1] driver core: Take parent lock in async device attach

[PATCH v1] driver core: Take parent lock in async device attach

[Mario Peter]

On buses with need_parent_lock set (only USB), 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 USB drivers (driver_async_probe=*,
module.async_probe=1), hub_probe() of a multi-TT hub then races with
usb_set_configuration() and both create the interface's endpoint
devices. On an i.MX8MM board 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

Take the parent lock there 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.

Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
Assisted-by: LLM
Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
---
Seen and tested on 6.16.y, which has the same code here. On mainline
only build-tested.

Not covered: deferred_probe_work_func() also re-probes via
__device_attach() without the parent lock.

Analysis and patch done with the help of Claude Code.

 drivers/base/dd.c | 68 +++++++++++++++++++++++------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index f6525a7ee8c5..a8912e6d5fbc 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -1029,6 +1029,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 bus 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 (parent && dev->bus->need_parent_lock)
+		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 bus 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 (parent && dev->bus->need_parent_lock)
+		device_unlock(parent);
+}
+
 static void __device_attach_async_helper(void *_dev, async_cookie_t cookie)
 {
 	struct device *dev = _dev;
@@ -1038,7 +1070,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
@@ -1060,7 +1092,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);
 }
@@ -1155,38 +1187,6 @@ void device_initial_probe(struct device *dev)
 	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 bus 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 (parent && dev->bus->need_parent_lock)
-		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 bus 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 (parent && dev->bus->need_parent_lock)
-		device_unlock(parent);
-}
-
 /**
  * device_driver_attach - attach a specific driver to a specific device
  * @drv: Driver to attach

base-commit: 22430ae5d90ab288b0ee2ad99ae941f4a666b694
-- 
2.43.0

Re: [PATCH v1] driver core: Take parent lock in async device attach

[Greg Kroah-Hartman]

On Tue, Oct 06, 2026 at 01:23:35PM +0000, Mario Peter wrote:
> On buses with need_parent_lock set (only USB), 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 USB drivers (driver_async_probe=*,
> module.async_probe=1), hub_probe() of a multi-TT hub then races with
> usb_set_configuration() and both create the interface's endpoint
> devices. On an i.MX8MM board 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
> 
> Take the parent lock there 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.
> 
> Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
> Assisted-by: LLM
> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
> ---
> Seen and tested on 6.16.y, which has the same code here. On mainline
> only build-tested.

Please verify this on the latest tree, 6.16.y is _VERY_ old and
obsolete, lots has changed in the year it was released.

> Not covered: deferred_probe_work_func() also re-probes via
> __device_attach() without the parent lock.
> 
> Analysis and patch done with the help of Claude Code.

This feels really wrong, what bus is the host controller on for these?

thanks,

greg k-h

Re: [PATCH v1] driver core: Take parent lock in async device attach

[PETER Mario]

Hi Greg,

On 10/6/26 16:38, Greg Kroah-Hartman wrote:

> On Tue, Oct 06, 2026 at 01:23:35PM +0000, Mario Peter wrote:
>> On buses with need_parent_lock set (only USB), 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 USB drivers (driver_async_probe=*,
>> module.async_probe=1), hub_probe() of a multi-TT hub then races with
>> usb_set_configuration() and both create the interface's endpoint
>> devices. On an i.MX8MM board 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
>>
>> Take the parent lock there 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.
>>
>> Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
>> Assisted-by: LLM
>> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
>> ---
>> Seen and tested on 6.16.y, which has the same code here. On mainline
>> only build-tested.
> 
> Please verify this on the latest tree, 6.16.y is _VERY_ old and
> obsolete, lots has changed in the year it was released.

Moving this board to the latest kernel for a test isn't that easy, as
its board support isn't upstream. But the affected code is still the
same in v7.3-rc6. In dd.c, __device_attach_async_helper(),
__device_attach() and __driver_attach_async_helper() are unchanged
since v6.16. On the USB side, usb_set_interface() and
create_intf_ep_devs() are unchanged, and usb_set_configuration() and
hub_configure() only got the kmalloc_obj() conversions.

>> Not covered: deferred_probe_work_func() also re-probes via
>> __device_attach() without the parent lock.
>>
>> Analysis and patch done with the help of Claude Code.
> 
> This feels really wrong, what bus is the host controller on for these?

The platform bus, it's the ChipIdea controller of the i.MX8MM:
   /sys/devices/platform/soc@0/32c00000.bus/32e50000.usb/ci_hdrc.1/usb1/1-1/1-1:1.0

1-1 is an onboard USB2514 hub (multi-TT). The host controller isn't
part of the race, both sides are in the USB core, for that one hub:
  
- usb_generic_driver_probe() of 1-1 calls usb_set_configuration(),
  which holds the 1-1 lock, does device_add() for 1-1:1.0 and then
  create_intf_ep_devs() for it.
     
- device_add() only queues the probe of 1-1:1.0. The async worker runs
  hub_probe() -> usb_set_interface(hdev, 0, 1) -> create_intf_ep_devs()
  with only the 1-1:1.0 lock held.
     
create_intf_ep_devs() checks and sets intf->ep_devs_created without a
lock of its own, so both create ep_81. With a synchronous probe,
hub_probe() runs inside device_add() with the 1-1 lock held, and this
can't happen.

The USB core relies on that lock. need_parent_lock is documented as
"When probing or removing a device on this bus, the device core should
lock the device's parent", and usb_driver_claim_interface() says
"Callers must own the device lock, so driver probe() entries don't need
extra locking". __driver_attach_async_helper() takes the parent lock,
__device_attach_async_helper() doesn't.
  
Thanks,
Mario


> 
> thanks,
> 
> greg k-h

Re: [PATCH v1] driver core: Take parent lock in async device attach

[Alan Stern]

On Tue, Oct 06, 2026 at 03:29:48PM +0000, PETER Mario wrote:
> Hi Greg,
> 
> On 10/6/26 16:38, Greg Kroah-Hartman wrote:
> 
> > On Tue, Oct 06, 2026 at 01:23:35PM +0000, Mario Peter wrote:
> >> On buses with need_parent_lock set (only USB), 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 USB drivers (driver_async_probe=*,
> >> module.async_probe=1), hub_probe() of a multi-TT hub then races with
> >> usb_set_configuration() and both create the interface's endpoint
> >> devices. On an i.MX8MM board 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
> >>
> >> Take the parent lock there 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.
> >>
> >> Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
> >> Assisted-by: LLM
> >> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
> >> ---
> >> Seen and tested on 6.16.y, which has the same code here. On mainline
> >> only build-tested.
> > 
> > Please verify this on the latest tree, 6.16.y is _VERY_ old and
> > obsolete, lots has changed in the year it was released.
> 
> Moving this board to the latest kernel for a test isn't that easy, as
> its board support isn't upstream.

It should be fairly simple to run the verification on a standard PC 
using an up-to-date kernel.  Nothing in the bug description or fix is 
specific to i.MX8MM.

> But the affected code is still the
> same in v7.3-rc6. In dd.c, __device_attach_async_helper(),
> __device_attach() and __driver_attach_async_helper() are unchanged
> since v6.16. On the USB side, usb_set_interface() and
> create_intf_ep_devs() are unchanged, and usb_set_configuration() and
> hub_configure() only got the kmalloc_obj() conversions.
> 
> >> Not covered: deferred_probe_work_func() also re-probes via
> >> __device_attach() without the parent lock.
> >>
> >> Analysis and patch done with the help of Claude Code.
> > 
> > This feels really wrong, what bus is the host controller on for these?
> 
> The platform bus, it's the ChipIdea controller of the i.MX8MM:
>    /sys/devices/platform/soc@0/32c00000.bus/32e50000.usb/ci_hdrc.1/usb1/1-1/1-1:1.0
> 
> 1-1 is an onboard USB2514 hub (multi-TT). The host controller isn't

Does the fact that the onboard hub is multi-TT have any connection with 
the bug?  Not as far as I can see -- but I had to waste a minute 
thinking about it.  If you agree, please remove that irrelevant detail 
from the patch description.

> part of the race, both sides are in the USB core, for that one hub:
>   
> - usb_generic_driver_probe() of 1-1 calls usb_set_configuration(),
>   which holds the 1-1 lock, does device_add() for 1-1:1.0 and then
>   create_intf_ep_devs() for it.
>      
> - device_add() only queues the probe of 1-1:1.0. The async worker runs
>   hub_probe() -> usb_set_interface(hdev, 0, 1) -> create_intf_ep_devs()
>   with only the 1-1:1.0 lock held.
>      
> create_intf_ep_devs() checks and sets intf->ep_devs_created without a
> lock of its own, so both create ep_81. With a synchronous probe,
> hub_probe() runs inside device_add() with the 1-1 lock held, and this
> can't happen.

You should describe this race in more detail (like you just did here) in 
the patch description.  It will help explain exactly what it is you are 
fixing.

> The USB core relies on that lock. need_parent_lock is documented as
> "When probing or removing a device on this bus, the device core should
> lock the device's parent", and usb_driver_claim_interface() says
> "Callers must own the device lock, so driver probe() entries don't need
> extra locking". __driver_attach_async_helper() takes the parent lock,
> __device_attach_async_helper() doesn't.

FWIW, I agree that this is a real bug and your solution is the right 
approach for fixing it.

Alan Stern

Re: [PATCH v1] driver core: Take parent lock in async device attach

[PETER Mario]

Hi Alan,

On 10/6/26 22:45, Alan Stern wrote: 
> 
> On Tue, Oct 06, 2026 at 03:29:48PM +0000, PETER Mario wrote:
>> Hi Greg,
>>
>> On 10/6/26 16:38, Greg Kroah-Hartman wrote:
>>
>>> On Tue, Oct 06, 2026 at 01:23:35PM +0000, Mario Peter wrote:
>>>> On buses with need_parent_lock set (only USB), 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 USB drivers (driver_async_probe=*,
>>>> module.async_probe=1), hub_probe() of a multi-TT hub then races with
>>>> usb_set_configuration() and both create the interface's endpoint
>>>> devices. On an i.MX8MM board 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
>>>>
>>>> Take the parent lock there 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.
>>>>
>>>> Fixes: 765230b5f084 ("driver-core: add asynchronous probing support for drivers")
>>>> Assisted-by: LLM
>>>> Signed-off-by: Mario Peter <mario.peter@leica-geosystems.com>
>>>> ---
>>>> Seen and tested on 6.16.y, which has the same code here. On mainline
>>>> only build-tested.
>>>
>>> Please verify this on the latest tree, 6.16.y is _VERY_ old and
>>> obsolete, lots has changed in the year it was released.
>>
>> Moving this board to the latest kernel for a test isn't that easy, as
>> its board support isn't upstream.
> 
> It should be fairly simple to run the verification on a standard PC
> using an up-to-date kernel.  Nothing in the bug description or fix is
> specific to i.MX8MM.

Right. I reproduced it on v7.3-rc6 in QEMU, booted with
driver_async_probe=hub, with dummy_hcd and a small raw-gadget program
that emulates a multi-TT hub. A loop writes 0 and 1 to the hub's
"authorized" attribute. Each time, usb_set_configuration() runs in the
writing task with the udev lock held, and hub_probe() runs in the
async worker, the same two paths as in the trace from the board.

The window is small: usb_set_configuration() has to stall after it
has created ep_81 but before it sets ep_devs_created, for longer than
hub_probe() needs to get to usb_set_interface(). To make that happen
often enough, a SCHED_FIFO task preempts the writing task at random
points for 3 ms. With that, the warning hit 29 of 6000 iterations
without the patch, with the same call trace as on the board, and 0 of
6000 with it.

Independent of the timing, a debug-only device_lock_assert(&udev->dev)
in usb_probe_interface() fired on every async probe of a hub interface
without the patch (52 of 52) and never with it (0 of 102).

I can post the emulator and the test scripts if that helps.

>> But the affected code is still the
>> same in v7.3-rc6. In dd.c, __device_attach_async_helper(),
>> __device_attach() and __driver_attach_async_helper() are unchanged
>> since v6.16. On the USB side, usb_set_interface() and
>> create_intf_ep_devs() are unchanged, and usb_set_configuration() and
>> hub_configure() only got the kmalloc_obj() conversions.
>>
>>>> Not covered: deferred_probe_work_func() also re-probes via
>>>> __device_attach() without the parent lock.
>>>>
>>>> Analysis and patch done with the help of Claude Code.
>>>
>>> This feels really wrong, what bus is the host controller on for these?
>>
>> The platform bus, it's the ChipIdea controller of the i.MX8MM:
>>    /sys/devices/platform/soc@0/32c00000.bus/32e50000.usb/ci_hdrc.1/usb1/1-1/1-1:1.0
>>
>> 1-1 is an onboard USB2514 hub (multi-TT). The host controller isn't
> 
> Does the fact that the onboard hub is multi-TT have any connection with
> the bug?  Not as far as I can see -- but I had to waste a minute
> thinking about it.  If you agree, please remove that irrelevant detail
> from the patch description.

Only with this symptom. For multi-TT hubs, hub_configure() calls
usb_set_interface(hdev, 0, 1) to select the TT-per-port altsetting,
and that's the call that creates the endpoint devices a second time
from hub_probe(). A single-TT hub doesn't make that call, so it won't
show this warning, but its probe still runs without the udev lock.

The missing lock itself isn't hub specific, so in v2 I'll say why the
hub has to be multi-TT instead of just naming it.

>> part of the race, both sides are in the USB core, for that one hub:
>>
>> - usb_generic_driver_probe() of 1-1 calls usb_set_configuration(),
>>   which holds the 1-1 lock, does device_add() for 1-1:1.0 and then
>>   create_intf_ep_devs() for it.
>>
>> - device_add() only queues the probe of 1-1:1.0. The async worker runs
>>   hub_probe() -> usb_set_interface(hdev, 0, 1) -> create_intf_ep_devs()
>>   with only the 1-1:1.0 lock held.
>>
>> create_intf_ep_devs() checks and sets intf->ep_devs_created without a
>> lock of its own, so both create ep_81. With a synchronous probe,
>> hub_probe() runs inside device_add() with the 1-1 lock held, and this
>> can't happen.
> 
> You should describe this race in more detail (like you just did here) in
> the patch description.  It will help explain exactly what it is you are
> fixing.

Will do in v2.

>> The USB core relies on that lock. need_parent_lock is documented as
>> "When probing or removing a device on this bus, the device core should
>> lock the device's parent", and usb_driver_claim_interface() says
>> "Callers must own the device lock, so driver probe() entries don't need
>> extra locking". __driver_attach_async_helper() takes the parent lock,
>> __device_attach_async_helper() doesn't.
> 
> FWIW, I agree that this is a real bug and your solution is the right
> approach for fixing it.
> 
> Alan Stern

Thanks for the review.  
Mario