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