Commit 18daa524 authored by Dmitry Torokhov's avatar Dmitry Torokhov Committed by Greg Kroah-Hartman
Browse files

driver core: fix potential NULL pointer dereference in dev_uevent() 



If userspace reads "uevent" device attribute at the same time as another
threads unbinds the device from its driver, change to dev->driver from a
valid pointer to NULL may result in crash. Fix this by using READ_ONCE()
when fetching the pointer, and take bus' drivers klist lock to make sure
driver instance will not disappear while we access it.

Use WRITE_ONCE() when setting the driver pointer to ensure there is no
tearing.

Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
Reviewed-by: default avatarMasami Hiramatsu (Google) <mhiramat@kernel.org>
Link: https://lore.kernel.org/r/20250311052417.1846985-3-dmitry.torokhov@gmail.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 04d3e546

drivers/base/base.h

+12 −1
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ static inline void subsys_put(struct subsys_private *sp) 
		kset_put(&sp->subsys);

}



struct subsys_private *bus_to_subsys(const struct bus_type *bus);

struct subsys_private *class_to_subsys(const struct class *class);



struct driver_private {
@@ -182,8 +183,18 @@ void device_driver_detach(struct device *dev); 


static inline void device_set_driver(struct device *dev, const struct device_driver *drv)

{

	/*

	 * Majority (all?) read accesses to dev->driver happens either

	 * while holding device lock or in bus/driver code that is only

	 * invoked when the device is bound to a driver and there is no

	 * concern of the pointer being changed while it is being read.

	 * However when reading device's uevent file we read driver pointer

	 * without taking device lock (so we do not block there for

	 * arbitrary amount of time). We use WRITE_ONCE() here to prevent

	 * tearing so that READ_ONCE() can safely be used in uevent code.

	 */

	// FIXME - this cast should not be needed "soon"

	dev->driver = (struct device_driver *)drv;

	WRITE_ONCE(dev->driver, (struct device_driver *)drv);

}



int devres_release_all(struct device *dev);

drivers/base/bus.c

+1 −1
Original line number Diff line number Diff line
@@ -57,7 +57,7 @@ static int __must_check bus_rescan_devices_helper(struct device *dev, 
 * NULL.  A call to subsys_put() must be done when finished with the pointer in

 * order for it to be properly freed.

 */

static struct subsys_private *bus_to_subsys(const struct bus_type *bus)

struct subsys_private *bus_to_subsys(const struct bus_type *bus)

{

	struct subsys_private *sp = NULL;

	struct kobject *kobj;

drivers/base/core.c

+31 −2
Original line number Diff line number Diff line
@@ -2624,6 +2624,35 @@ static const char *dev_uevent_name(const struct kobject *kobj) 
	return NULL;

}



/*

 * Try filling "DRIVER=<name>" uevent variable for a device. Because this

 * function may race with binding and unbinding the device from a driver,

 * we need to be careful. Binding is generally safe, at worst we miss the

 * fact that the device is already bound to a driver (but the driver

 * information that is delivered through uevents is best-effort, it may

 * become obsolete as soon as it is generated anyways). Unbinding is more

 * 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.

 */

static void dev_driver_uevent(const struct device *dev, struct kobj_uevent_env *env)

{

	struct subsys_private *sp = bus_to_subsys(dev->bus);



	if (sp) {

		scoped_guard(spinlock, &sp->klist_drivers.k_lock) {

			struct device_driver *drv = READ_ONCE(dev->driver);

			if (drv)

				add_uevent_var(env, "DRIVER=%s", drv->name);

		}



		subsys_put(sp);

	}

}



static int dev_uevent(const struct kobject *kobj, struct kobj_uevent_env *env)

{

	const struct device *dev = kobj_to_dev(kobj);
@@ -2655,8 +2684,8 @@ static int dev_uevent(const struct kobject *kobj, struct kobj_uevent_env *env) 
	if (dev->type && dev->type->name)

		add_uevent_var(env, "DEVTYPE=%s", dev->type->name);



	if (dev->driver)

		add_uevent_var(env, "DRIVER=%s", dev->driver->name);

	/* Add "DRIVER=%s" variable if the device is bound to a driver */

	dev_driver_uevent(dev, env);



	/* Add common DT information about the device */

	of_device_uevent(dev, env);