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@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> 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