USB: fix possible deadlock involving sysfs attributes
authorAlan Stern <stern@rowland.harvard.edu>
Tue, 20 May 2008 20:37:34 +0000 (16:37 -0400)
committerGreg Kroah-Hartman <gregkh@suse.de>
Thu, 29 May 2008 20:59:03 +0000 (13:59 -0700)
There is a potential deadlock when the usb_generic driver is unbound
from a device.  The problem is that generic_disconnect() is called
with the device lock held, and it removes a bunch of device attributes
from sysfs.  If a user task happens to be running an attribute method
at the time, the removal will block until the method returns.  But at
least one of the attribute methods (the store routine for power/level)
needs to acquire the device lock!

This patch (as1093) eliminates the deadlock by moving the calls to
create and remove the sysfs attributes from the usb_generic driver
into usb_new_device() and usb_disconnect(), where they can be invoked
without holding the device lock.

Besides, the other sysfs attributes are created when the device is
registered and removed when the device is unregistered.  So it seems
only fitting for the extra attributes to be created and removed at the
same time.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/core/generic.c
drivers/usb/core/hub.c

index c1cb94e9f242a7fe2806e179b05122577b03b62f..7e912f21fd365fc7240bc8c698c43ce2a817cc6b 100644 (file)
@@ -155,9 +155,6 @@ static int generic_probe(struct usb_device *udev)
 {
        int err, c;
 
-       /* put device-specific files into sysfs */
-       usb_create_sysfs_dev_files(udev);
-
        /* Choose and set the configuration.  This registers the interfaces
         * with the driver core and lets interface drivers bind to them.
         */
@@ -189,8 +186,6 @@ static void generic_disconnect(struct usb_device *udev)
         * unconfigure the device */
        if (udev->actconfig)
                usb_set_configuration(udev, -1);
-
-       usb_remove_sysfs_dev_files(udev);
 }
 
 #ifdef CONFIG_PM
index eb57fcc701d7d995e271c39773f0ac3738214222..1a3d2879bc1d4ee0e1856c2862a37a6f58148341 100644 (file)
@@ -1326,6 +1326,12 @@ void usb_disconnect(struct usb_device **pdev)
 
        usb_unlock_device(udev);
 
+       /* Remove the device-specific files from sysfs.  This must be
+        * done with udev unlocked, because some of the attribute
+        * routines try to acquire the device lock.
+        */
+       usb_remove_sysfs_dev_files(udev);
+
        /* Unregister the device.  The device driver is responsible
         * for removing the device files from usbfs and sysfs and for
         * de-configuring the device.
@@ -1541,6 +1547,9 @@ int usb_new_device(struct usb_device *udev)
                goto fail;
        }
 
+       /* put device-specific files into sysfs */
+       usb_create_sysfs_dev_files(udev);
+
        /* Tell the world! */
        announce_device(udev);
        return err;