The patch titled
driver core: fix bus_rescan_devices() race
has been added to the -mm tree. Its filename is
driver-core-fix-bus_rescan_devices-race.patch
Patches currently in -mm which might be from [EMAIL PROTECTED] are
driver-core-fix-bus_rescan_devices-race.patch
yenta-make-topic95-bridges-work-with-16bit-cards.patch
pcmcia-cs-fix-possible-missed-wakeup.patch
fix-pcmcia_request_irq-for-multifunction-card.patch
pcmcia-yenta-avoid-pci-write-posting-problem.patch
yenta-auto-tune-ene-bridges-for-cb-cards.patch
From: Daniel Ritz <[EMAIL PROTECTED]>
bus_rescan_devices_helper() does not hold the dev->sem when it checks for
!dev->driverOP. device_attach() holds the sem, but calls again
device_bind_driver() even when dev->driver is set.
What happens is that a first device_attach() call (module insertion time)
is on the way binding the device to a driver. Another thread calls
bus_rescan_devices(). Now when bus_rescan_devices_helper() checks for
dev->driver it is still NULL 'cos the the prior device_attach() is not yet
finished. But as soon as the first one releases the dev->sem the second
device_attach() tries to rebind the already bound device again.
device_bind_driver() does this blindly which leads to a corrupt
driver->klist_devices list (the device links itself, the head points to the
device). Later a call to device_release_driver() sets dev->driver to NULL
and breaks the link it has to itself on knode_driver. Rmmoding the driver
later calls driver_detach() which leads to an endless loop 'cos the list
head in klist_devices still points to the device. And since dev->driver is
NULL it's stuck with the same device forever. Boom. And rmmod hangs.
Very easy to reproduce with new-style pcmcia and a 16bit card. Just loop
modprobe <pcmcia-modules> ;cardctl eject; rmmod <card driver, pcmcia modules>.
Easiest fix is to check if the device is already bound to a driver in
device_bind_driver(). This avoids the double binding. Nicer would be not to
call device_bind_driver() in device_attach() if dev->driver is set. But since
some drivers are messing with dev->driver directly this is not a good idea.
And while at it replace spin_(un|)lock_irq in driver_detach with the
non-irq variants. Just doesn't make sense to me. The whole klist locking
never uses the irq variants.
Signed-off-by: Daniel Ritz <[EMAIL PROTECTED]>
Cc: Greg KH <[EMAIL PROTECTED]>
Cc: Dominik Brodowski <[EMAIL PROTECTED]>
Signed-off-by: Andrew Morton <[EMAIL PROTECTED]>
---
drivers/base/bus.c | 3 +--
drivers/base/dd.c | 9 ++++++---
2 files changed, 7 insertions(+), 5 deletions(-)
diff -puN drivers/base/bus.c~driver-core-fix-bus_rescan_devices-race
drivers/base/bus.c
--- 25/drivers/base/bus.c~driver-core-fix-bus_rescan_devices-race Mon Aug
29 14:07:40 2005
+++ 25-akpm/drivers/base/bus.c Mon Aug 29 14:07:40 2005
@@ -485,8 +485,7 @@ void bus_remove_driver(struct device_dri
/* Helper for bus_rescan_devices's iter */
static int bus_rescan_devices_helper(struct device *dev, void *data)
{
- if (!dev->driver)
- device_attach(dev);
+ device_attach(dev);
return 0;
}
diff -puN drivers/base/dd.c~driver-core-fix-bus_rescan_devices-race
drivers/base/dd.c
--- 25/drivers/base/dd.c~driver-core-fix-bus_rescan_devices-race Mon Aug
29 14:07:40 2005
+++ 25-akpm/drivers/base/dd.c Mon Aug 29 14:07:40 2005
@@ -40,6 +40,9 @@
*/
void device_bind_driver(struct device * dev)
{
+ if (klist_node_attached(&dev->knode_driver))
+ return;
+
pr_debug("bound device '%s' to driver '%s'\n",
dev->bus_id, dev->driver->name);
klist_add_tail(&dev->knode_driver, &dev->driver->klist_devices);
@@ -222,15 +225,15 @@ void driver_detach(struct device_driver
struct device * dev;
for (;;) {
- spin_lock_irq(&drv->klist_devices.k_lock);
+ spin_lock(&drv->klist_devices.k_lock);
if (list_empty(&drv->klist_devices.k_list)) {
- spin_unlock_irq(&drv->klist_devices.k_lock);
+ spin_unlock(&drv->klist_devices.k_lock);
break;
}
dev = list_entry(drv->klist_devices.k_list.prev,
struct device, knode_driver.n_node);
get_device(dev);
- spin_unlock_irq(&drv->klist_devices.k_lock);
+ spin_unlock(&drv->klist_devices.k_lock);
down(&dev->sem);
if (dev->driver == drv)
_
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
