Re: [PATCH v1] driver core: unbind only the driver whose unbind file was written

[Greg Kroah-Hartman]

On Fri, Oct 16, 2026 at 10:09:35PM -0400, Nickolai Zeldovich wrote:
> unbind_store() checks dev->driver == drv without the device lock and
> then calls device_driver_detach(), which is
> device_release_driver_internal(dev, NULL, dev->parent): with a NULL
> driver that function releases whichever driver is bound once it holds
> the lock.  Between the check and the lock another thread can unbind
> drv and bind a different driver to the same device (a second write to
> the unbind and bind files, a driver_override write followed by
> drivers_probe, or the bus re-probing the device after the first
> unbind), and the write to drv's unbind file then detaches the other
> driver, one the writer never named, and reports success.

bind/unbind now causes a taint, so what is the real problem here?  A
root user is poking around and causing problems, so any error is their
fault for doing this?

> The pattern is as old as the unbind file: the check was never under the
> lock and the release never named the driver.  The locked variant has
> existed since commit 4bdb35506b89 ("driver core: Add a wrapper around
> __device_release_driver()") added the drv argument to
> device_release_driver_internal() for driver_detach(), which has the
> same requirement.

Because this has always been present, and no one has actually reported
this, perhaps it is not really a problem at all?  In other words, what
real-life-use requires this?  Again, bind/unbind is for developers only.

> Pass drv to device_release_driver_internal(): it re-checks dev->driver
> under the lock and releases the driver only when it is still the one
> the file belongs to.  When the driver changed in between, the write
> detaches nothing; drv is no longer bound to the device either way, so
> the write still returns the count.
> 
> Found by formal verification of drivers/base/bus.c.

Where is the formal verification proof?  Please don't have your LLM
claim this without that as it's pointless.

> Fixes: 151ef38f7c0e ("[PATCH] driver core: Add the ability to unbind drivers to devices from userspace")
> Assisted-by: LLM
> Signed-off-by: Nickolai Zeldovich <nickolai@xxxxxxxxxxxxx>
> ---
> 
> Notes:
>     The unlocked dev->driver == drv test stays as the fast path (a device
>     bound to another driver still gets -ENODEV there); making the write
>     return -ENODEV when only the locked re-check fails would need a return
>     value from device_release_driver_internal(), which three other callers
>     ignore, and the writer's aim, drv no longer bound to the device, is met
>     in that case. The race window is the time
>     between the test and __device_driver_lock(); it was not hit in testing
>     (no in-tree pair of drivers with a slow probe was raced), the
>     interleaving is described in the message and a userspace racer is in
>     the review material. bind_store() has no such hole: __driver_probe_device()
>     checks dev->driver under the lock.

I don't understand, this is not a place where a "race" can happen.

thanks,

greg k-h