aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAlan Stern <stern@rowland.harvard.edu>2006-09-18 16:22:34 -0400
committerGreg Kroah-Hartman <gregkh@suse.de>2006-09-26 00:08:40 -0400
commitf2eaae197f4590c4d96f31b09b0ee9067421a95c (patch)
tree5ccb072851da5d1bfdae458c52b57e76eb7dd168
parent0f397f865076e3471ec884ee73ad5e34165fac2a (diff)
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 <stern@rowland.harvard.edu> Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r--drivers/base/bus.c8
-rw-r--r--drivers/base/dd.c2
-rw-r--r--include/linux/device.h3
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:
392 * bus_attach_device - add device to bus 392 * bus_attach_device - add device to bus
393 * @dev: device tried to attach to a driver 393 * @dev: device tried to attach to a driver
394 * 394 *
395 * - Add device to bus's list of devices.
395 * - Try to attach to driver. 396 * - Try to attach to driver.
396 */ 397 */
397int bus_attach_device(struct device * dev) 398int bus_attach_device(struct device * dev)
@@ -400,11 +401,13 @@ int bus_attach_device(struct device * dev)
400 int ret = 0; 401 int ret = 0;
401 402
402 if (bus) { 403 if (bus) {
404 dev->is_registered = 1;
403 ret = device_attach(dev); 405 ret = device_attach(dev);
404 if (ret >= 0) { 406 if (ret >= 0) {
405 klist_add_tail(&dev->knode_bus, &bus->klist_devices); 407 klist_add_tail(&dev->knode_bus, &bus->klist_devices);
406 ret = 0; 408 ret = 0;
407 } 409 } else
410 dev->is_registered = 0;
408 } 411 }
409 return ret; 412 return ret;
410} 413}
@@ -425,7 +428,8 @@ void bus_remove_device(struct device * dev)
425 sysfs_remove_link(&dev->kobj, "bus"); 428 sysfs_remove_link(&dev->kobj, "bus");
426 sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id); 429 sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
427 device_remove_attrs(dev->bus, dev); 430 device_remove_attrs(dev->bus, dev);
428 klist_remove(&dev->knode_bus); 431 dev->is_registered = 0;
432 klist_del(&dev->knode_bus);
429 pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id); 433 pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
430 device_release_driver(dev); 434 device_release_driver(dev);
431 put_bus(dev->bus); 435 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)
162 struct task_struct *probe_task; 162 struct task_struct *probe_task;
163 int ret = 0; 163 int ret = 0;
164 164
165 if (!device_is_registered(dev))
166 return -ENODEV;
165 if (drv->bus->match && !drv->bus->match(dev, drv)) 167 if (drv->bus->match && !drv->bus->match(dev, drv))
166 goto done; 168 goto done;
167 169
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 {
329 329
330 struct kobject kobj; 330 struct kobject kobj;
331 char bus_id[BUS_ID_SIZE]; /* position on parent bus */ 331 char bus_id[BUS_ID_SIZE]; /* position on parent bus */
332 unsigned is_registered:1;
332 struct device_attribute uevent_attr; 333 struct device_attribute uevent_attr;
333 struct device_attribute *devt_attr; 334 struct device_attribute *devt_attr;
334 335
@@ -381,7 +382,7 @@ dev_set_drvdata (struct device *dev, void *data)
381 382
382static inline int device_is_registered(struct device *dev) 383static inline int device_is_registered(struct device *dev)
383{ 384{
384 return klist_node_attached(&dev->knode_bus); 385 return dev->is_registered;
385} 386}
386 387
387/* 388/*