linux-kernel - [PATCH 45/47] Driver core: Fix potential deadlock in driver core

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <115924922922-git-send-email-greg@kroah.com>
Date:	Mon, 25 Sep 2006 22:38:05 -0700
From:	Greg KH <greg@...ah.com>
To:	linux-kernel@...r.kernel.org
Cc:	Alan Stern <stern@...land.harvard.edu>,
	Greg Kroah-Hartman <gregkh@...e.de>
Subject: [PATCH 45/47] Driver core: Fix potential deadlock in driver core

From: Alan Stern <stern@...land.harvard.edu>

There is a potential deadlock in the driver core.  It boils down to
the fact that bus_remove_device() calls klist_remove() instead of
klist_del(), thereby waiting until the reference count of the
klist_node in the bus's klist of devices drops to 0.  The refcount
can't reach 0 so long as a modprobe process is trying to bind a new
driver to the device being removed, by calling __driver_attach().  The
problem is that __driver_attach() tries to acquire the device's
parent's semaphore, but the caller of bus_remove_device() is quite
likely to own that semaphore already.

It isn't sufficient just to replace klist_remove() with klist_del().
Doing so runs the risk that the device would remain on the bus's klist
of devices for some time, and so could be bound to another driver even
after it was unregistered.  What's needed is a new way to distinguish
whether or not a device is registered, based on a criterion other than
whether its klist_node is linked into the bus's klist of devices.  That
way driver binding can fail when the device is unregistered, even if
it is still linked into the klist.

This patch (as782) implements the solution, by adding a new bitflag to
indiate when a struct device is registered, by testing the flag before
allowing a driver to bind a device, and by changing the definition of
the device_is_registered() inline.

Signed-off-by: Alan Stern <stern@...land.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>
---
 drivers/base/bus.c     |    8 ++++++--
 drivers/base/dd.c      |    2 ++
 include/linux/device.h |    3 ++-
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index aa685a2..636af53 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -392,6 +392,7 @@ out:
  *	bus_attach_device - add device to bus
  *	@dev:	device tried to attach to a driver
  *
+ *	- Add device to bus's list of devices.
  *	- Try to attach to driver.
  */
 int bus_attach_device(struct device * dev)
@@ -400,11 +401,13 @@ int bus_attach_device(struct device * de
 	int ret = 0;

 	if (bus) {
+		dev->is_registered = 1;
 		ret = device_attach(dev);
 		if (ret >= 0) {
 			klist_add_tail(&dev->knode_bus, &bus->klist_devices);
 			ret = 0;
-		}
+		} else
+			dev->is_registered = 0;
 	}
 	return ret;
 }
@@ -425,7 +428,8 @@ void bus_remove_device(struct device * d
 		sysfs_remove_link(&dev->kobj, "bus");
 		sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
 		device_remove_attrs(dev->bus, dev);
-		klist_remove(&dev->knode_bus);
+		dev->is_registered = 0;
+		klist_del(&dev->knode_bus);
 		pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
 		device_release_driver(dev);
 		put_bus(dev->bus);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 319a73b..b5f43c3 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -162,6 +162,8 @@ int driver_probe_device(struct device_dr
 	struct task_struct *probe_task;
 	int ret = 0;

+	if (!device_is_registered(dev))
+		return -ENODEV;
 	if (drv->bus->match && !drv->bus->match(dev, drv))
 		goto done;

diff --git a/include/linux/device.h b/include/linux/device.h
index 74246ef..662e6a1 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -329,6 +329,7 @@ struct device {

 	struct kobject kobj;
 	char	bus_id[BUS_ID_SIZE];	/* position on parent bus */
+	unsigned		is_registered:1;
 	struct device_attribute uevent_attr;
 	struct device_attribute *devt_attr;

@@ -381,7 +382,7 @@ dev_set_drvdata (struct device *dev, voi

 static inline int device_is_registered(struct device *dev)
 {
-	return klist_node_attached(&dev->knode_bus);
+	return dev->is_registered;
 }

 /*
-- 
1.4.2.1

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/