Re: klists and struct device semaphores

[Alan Stern <stern@rowland.harvard.edu>]

Pat:

I looked through the new driver model code a bit more.  There appears to 
be a few problems (unless I'm using out-of-date code).

First, there's a race between adding a new device and registering a new 
driver.  The bus_add_device() routine contains these lines:

	pr_debug("bus %s: add device %s\n", bus->name, dev->bus_id);
	device_attach(dev);
	klist_add_tail(&bus->klist_devices, &dev->knode_bus);

Suppose device_attach() doesn't find a suitable driver, but a new driver 
is registered before the klist_add_tail() executes.  Then the new driver 
won't see the device either, and the device won't be bound at all.  The 
last two lines above should be in the opposite order.

Second, there's no check in driver_probe_device() or higher up to prevent
probing a device that's already bound to another driver.  Such a check
needs to be synchronized with assignments to dev->driver, so it should be
made while holding dev->sem.

Third, why does device_release_driver() call klist_del() instead of 
klist_remove() for dev->knode_driver?  Is that just a simple mistake?
The klist_node doesn't seem to get unlinked anywhere.

Fourth, in device_release_driver() why isn't most of the work done under
the protection of dev->sem?  If a driver is unregistered at the same time
as a device is removed, two threads could end up executing that routine at
the same time.  Then the question would be which thread calls
klist_remove() -- not to mention the danger that both of them might.  I 
guess the answer is to call klist_remove() after releasing dev->sem.

Alan Stern

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