[PATCH] driver core: fix race between bus_remove_driver() and dev_driver_uevent()

[PATCH] driver core: fix race between bus_remove_driver() and dev_driver_uevent()

[Dmitry Torokhov]

When a device driver is being removed, bus_remove_driver() removes the
driver from the bus klist before detaching it from bound devices. If
dev_driver_uevent() runs concurrently on another CPU, it may acquire the
bus klist lock after the driver has been removed from the list, read the
still-valid driver pointer from the device, and access the driver's name
while bus_remove_driver() proceeds to free the driver structure.

To close this race window, acquire and immediately release the bus
drivers klist lock after all devices have been detached. This ensures
that any in-flight uevent generation that observed the driver pointer
before device detachment completed finishes reading the driver name
before the driver structure is destroyed.

Fixes: 18daa52418e7 ("driver core: fix potential NULL pointer dereference in dev_uevent()")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Extremely unlikely to hit so I would not worry about backporting to
stable.

 drivers/base/bus.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index d17bd91490ee..8b3aaf451e4f 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -830,6 +830,15 @@ void bus_remove_driver(struct device_driver *drv)
 	driver_remove_file(drv, &driver_attr_uevent);
 	klist_remove(&drv->p->knode_bus);
 	driver_detach(drv);
+
+	/*
+	 * Ensure that any concurrent dev_driver_uevent() that observed
+	 * dev->driver before driver_detach() cleared it, finishes
+	 * accessing drv->name.
+	 */
+	scoped_guard(spinlock, &sp->klist_drivers.k_lock) {
+	}
+
 	module_remove_driver(drv);
 	kobject_put(&drv->p->kobj);
 
-- 
2.56.0.385.gd3acb90ef8-goog


-- 
Dmitry

Re: [PATCH] driver core: fix race between bus_remove_driver() and dev_driver_uevent()

[Greg Kroah-Hartman]

On Tue, Oct 13, 2026 at 08:24:20PM -0700, Dmitry Torokhov wrote:
> When a device driver is being removed, bus_remove_driver() removes the
> driver from the bus klist before detaching it from bound devices. If
> dev_driver_uevent() runs concurrently on another CPU, it may acquire the
> bus klist lock after the driver has been removed from the list, read the
> still-valid driver pointer from the device, and access the driver's name
> while bus_remove_driver() proceeds to free the driver structure.
> 
> To close this race window, acquire and immediately release the bus
> drivers klist lock after all devices have been detached. This ensures
> that any in-flight uevent generation that observed the driver pointer
> before device detachment completed finishes reading the driver name
> before the driver structure is destroyed.
> 
> Fixes: 18daa52418e7 ("driver core: fix potential NULL pointer dereference in dev_uevent()")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Extremely unlikely to hit so I would not worry about backporting to
> stable.
> 
>  drivers/base/bus.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index d17bd91490ee..8b3aaf451e4f 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -830,6 +830,15 @@ void bus_remove_driver(struct device_driver *drv)
>  	driver_remove_file(drv, &driver_attr_uevent);
>  	klist_remove(&drv->p->knode_bus);
>  	driver_detach(drv);
> +
> +	/*
> +	 * Ensure that any concurrent dev_driver_uevent() that observed
> +	 * dev->driver before driver_detach() cleared it, finishes
> +	 * accessing drv->name.
> +	 */
> +	scoped_guard(spinlock, &sp->klist_drivers.k_lock) {
> +	}

Shouldn't the compiler just no-op this away into nothing?

thanks,

greg k-h

Re: [PATCH] driver core: fix race between bus_remove_driver() and dev_driver_uevent()

[Dmitry Torokhov]

On Wed, Oct 14, 2026 at 07:31:19AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 13, 2026 at 08:24:20PM -0700, Dmitry Torokhov wrote:
> > When a device driver is being removed, bus_remove_driver() removes the
> > driver from the bus klist before detaching it from bound devices. If
> > dev_driver_uevent() runs concurrently on another CPU, it may acquire the
> > bus klist lock after the driver has been removed from the list, read the
> > still-valid driver pointer from the device, and access the driver's name
> > while bus_remove_driver() proceeds to free the driver structure.
> > 
> > To close this race window, acquire and immediately release the bus
> > drivers klist lock after all devices have been detached. This ensures
> > that any in-flight uevent generation that observed the driver pointer
> > before device detachment completed finishes reading the driver name
> > before the driver structure is destroyed.
> > 
> > Fixes: 18daa52418e7 ("driver core: fix potential NULL pointer dereference in dev_uevent()")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > 
> > Extremely unlikely to hit so I would not worry about backporting to
> > stable.
> > 
> >  drivers/base/bus.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > index d17bd91490ee..8b3aaf451e4f 100644
> > --- a/drivers/base/bus.c
> > +++ b/drivers/base/bus.c
> > @@ -830,6 +830,15 @@ void bus_remove_driver(struct device_driver *drv)
> >  	driver_remove_file(drv, &driver_attr_uevent);
> >  	klist_remove(&drv->p->knode_bus);
> >  	driver_detach(drv);
> > +
> > +	/*
> > +	 * Ensure that any concurrent dev_driver_uevent() that observed
> > +	 * dev->driver before driver_detach() cleared it, finishes
> > +	 * accessing drv->name.
> > +	 */
> > +	scoped_guard(spinlock, &sp->klist_drivers.k_lock) {
> > +	}
> 
> Shouldn't the compiler just no-op this away into nothing?

It better not. Why would it?

Thanks.

-- 
Dmitry

Re: [PATCH] driver core: fix race between bus_remove_driver() and dev_driver_uevent()

[Greg Kroah-Hartman]

On Tue, Oct 13, 2026 at 11:11:33PM -0700, Dmitry Torokhov wrote:
> On Wed, Oct 14, 2026 at 07:31:19AM +0200, Greg Kroah-Hartman wrote:
> > On Tue, Oct 13, 2026 at 08:24:20PM -0700, Dmitry Torokhov wrote:
> > > When a device driver is being removed, bus_remove_driver() removes the
> > > driver from the bus klist before detaching it from bound devices. If
> > > dev_driver_uevent() runs concurrently on another CPU, it may acquire the
> > > bus klist lock after the driver has been removed from the list, read the
> > > still-valid driver pointer from the device, and access the driver's name
> > > while bus_remove_driver() proceeds to free the driver structure.
> > > 
> > > To close this race window, acquire and immediately release the bus
> > > drivers klist lock after all devices have been detached. This ensures
> > > that any in-flight uevent generation that observed the driver pointer
> > > before device detachment completed finishes reading the driver name
> > > before the driver structure is destroyed.
> > > 
> > > Fixes: 18daa52418e7 ("driver core: fix potential NULL pointer dereference in dev_uevent()")
> > > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > ---
> > > 
> > > Extremely unlikely to hit so I would not worry about backporting to
> > > stable.
> > > 
> > >  drivers/base/bus.c | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> > > index d17bd91490ee..8b3aaf451e4f 100644
> > > --- a/drivers/base/bus.c
> > > +++ b/drivers/base/bus.c
> > > @@ -830,6 +830,15 @@ void bus_remove_driver(struct device_driver *drv)
> > >  	driver_remove_file(drv, &driver_attr_uevent);
> > >  	klist_remove(&drv->p->knode_bus);
> > >  	driver_detach(drv);
> > > +
> > > +	/*
> > > +	 * Ensure that any concurrent dev_driver_uevent() that observed
> > > +	 * dev->driver before driver_detach() cleared it, finishes
> > > +	 * accessing drv->name.
> > > +	 */
> > > +	scoped_guard(spinlock, &sp->klist_drivers.k_lock) {
> > > +	}
> > 
> > Shouldn't the compiler just no-op this away into nothing?
> 
> It better not. Why would it?

Sorry, early morning before my coffee had kicked in, I was thinking a
simple lock/unlock pair without any data in the middle might be compiled
away...

Still feels odd though I don't have a better idea.

greg k-h