On Mon, 25 Apr 2005, Roman Kagan wrote:
> > I see. This may not be a problem so much with klists as with usbcore.
> > That is, device_release_driver isn't protected against recursion, but
> > perhaps it doesn't need to be since usbcore can be changed to prevent
> > recursion in usb_driver_release_interface.
>
> Well for non-klist version the test !list_empty(&dev->driver_list) in
> usb_driver_release_interface() was a good enough guard against recursion
> into device_release_driver(), so I'm not sure your patch is needed.
But it is needed for the klist version. I didn't send a patch for that
version because I don't have the -mm kernel source handy. The analogous
change to the code should work, however.
> > The other problem I was thinking of is related to the one you found.
> > Since driver_detach doesn't immediately delete the device from its
> > driver's list, it's possible that another driver will bind to the device
> > first and thereby corrupt the list pointers.
>
> I'm not sure I get your point. The problem I spoke of was that with
> klists, device_release_driver() didn't take immediate effect when called
> from driver_detach(), due to an extra reference from the klist_iter in
> the loop. That had a nasty effect on usb_driver_release_interface() by
> making it think that the device was still associated with the driver
> when it already wasn't. But it certainly didn't make it look as though
> the device was dissociated from the driver _before_ it actually was, so
> another driver shouldn't be able to mess things up. Or did you mean
> anything else I've missed?
Probably this is something you've missed. Let's say a driver X bound to
device D is rmmod'ed at the same time that driver Y is insmod'ed. Y
iterates through the bus list looking for devices it can control, and
let's assume it reaches D while X's disconnect routine is running.
Now X has locked D->dev.sem, so Y blocks when trying to lock it.
Here's what happens:
The call to X->disconnect returns.
device_release_driver calls klist_del for D's knode_driver
but doesn't wait for the removal to complete. (It can't
wait, because the klist iterator keeps the klist_node active
until device_release_driver returns.)
device_release_driver releases D->dev.sem.
Assume at this point that Y starts running (preemption).
__driver_attach acquires D->dev.sem on behalf of Y.
driver_probe_device calls Y->probe, which returns 0.
device_bind_driver calls klist_add_tail for D. But D's
knode_driver is still on X's klist_devices!
Result: X's klist_devices is now corrupted.
Alan Stern
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
