Commit f2eaae197f4590c4d96f31b09b0ee9067421a95c

Authored by Alan Stern
Committed by Greg Kroah-Hartman
1 parent 0f397f8650

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>

Showing 3 changed files with 10 additions and 3 deletions Side-by-side Diff

... ... @@ -392,6 +392,7 @@
392 392 * bus_attach_device - add device to bus
393 393 * @dev: device tried to attach to a driver
394 394 *
  395 + * - Add device to bus's list of devices.
395 396 * - Try to attach to driver.
396 397 */
397 398 int bus_attach_device(struct device * dev)
398 399  
... ... @@ -400,11 +401,13 @@
400 401 int ret = 0;
401 402  
402 403 if (bus) {
  404 + dev->is_registered = 1;
403 405 ret = device_attach(dev);
404 406 if (ret >= 0) {
405 407 klist_add_tail(&dev->knode_bus, &bus->klist_devices);
406 408 ret = 0;
407   - }
  409 + } else
  410 + dev->is_registered = 0;
408 411 }
409 412 return ret;
410 413 }
... ... @@ -425,7 +428,8 @@
425 428 sysfs_remove_link(&dev->kobj, "bus");
426 429 sysfs_remove_link(&dev->bus->devices.kobj, dev->bus_id);
427 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 433 pr_debug("bus %s: remove device %s\n", dev->bus->name, dev->bus_id);
430 434 device_release_driver(dev);
431 435 put_bus(dev->bus);
... ... @@ -162,6 +162,8 @@
162 162 struct task_struct *probe_task;
163 163 int ret = 0;
164 164  
  165 + if (!device_is_registered(dev))
  166 + return -ENODEV;
165 167 if (drv->bus->match && !drv->bus->match(dev, drv))
166 168 goto done;
167 169  
include/linux/device.h
... ... @@ -329,6 +329,7 @@
329 329  
330 330 struct kobject kobj;
331 331 char bus_id[BUS_ID_SIZE]; /* position on parent bus */
  332 + unsigned is_registered:1;
332 333 struct device_attribute uevent_attr;
333 334 struct device_attribute *devt_attr;
334 335  
... ... @@ -381,7 +382,7 @@
381 382  
382 383 static 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 /*