[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

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

[Dmitry Torokhov]

Hi Greg,

On Wed, Oct 14, 2026 at 02:16:50PM +0200, Greg Kroah-Hartman wrote:
> 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:
> > > > @@ -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.

Just a gentle ping on this patch. Could it be picked up, or would you
prefer a plain spin_lock()/spin_unlock() pair instead of the empty
scoped_guard() block?

Thanks.

-- 
Dmitry

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

[Danilo Krummrich]

On Thu Nov 5, 2026 at 2:21 AM CET, Dmitry Torokhov wrote:
> Just a gentle ping on this patch. Could it be picked up, or would you
> prefer a plain spin_lock()/spin_unlock() pair instead of the empty
> scoped_guard() block?

Personally I like the scoped_guard(), but to me either is fine.

But I'd consider updating the comment above dev_driver_uevent() accordingly,
e.g.:

diff --git a/drivers/base/core.c b/drivers/base/core.c
index cbe05197d4a0..cc6a1016c251 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2705,9 +2705,12 @@ static const char *dev_uevent_name(const struct kobject *kobj)
  * risky as driver pointer is transitioning to NULL, so READ_ONCE() should
  * be used to make sure we are dealing with the same pointer, and to
  * ensure that driver structure is not going to disappear from under us
- * we take bus' drivers klist lock. The assumption that only registered
- * driver can be bound to a device, and to unregister a driver bus code
- * will take the same lock.
+ * we take the bus' drivers klist lock. During driver unregistration,
+ * bus_remove_driver() acquires and releases this lock after detaching
+ * all devices and clearing their driver pointers. This waits for readers
+ * that observed the old pointer to finish accessing the driver name
+ * before the driver can be freed. Readers acquiring the lock afterward
+ * cannot observe the removed driver.
  */
 static void dev_driver_uevent(const struct device *dev, struct kobj_uevent_env *env)
 {