From f2eaae197f4590c4d96f31b09b0ee9067421a95c Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Mon, 18 Sep 2006 16:22:34 -0400 Subject: [PATCH] Driver core: Fix potential deadlock in driver core 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 Signed-off-by: Greg Kroah-Hartman --- 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 aa685a20b649..636af538a2b5 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 * dev) 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 * dev) 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 319a73be4180..b5f43c3e44fa 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -162,6 +162,8 @@ int driver_probe_device(struct device_driver * drv, struct device * dev) 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 74246efba931..662e6a10144e 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, void *data) static inline int device_is_registered(struct device *dev) { - return klist_node_attached(&dev->knode_bus); + return dev->is_registered; } /*