USB: Fix race condition when removing host controllers
authorAlan Stern <stern@rowland.harvard.edu>
Wed, 26 Sep 2012 17:09:53 +0000 (13:09 -0400)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 26 Sep 2012 17:21:08 +0000 (10:21 -0700)
This patch (as1607) fixes a race that can occur if a USB host
controller is removed while a process is reading the
/sys/kernel/debug/usb/devices file.

The usb_device_read() routine uses the bus->root_hub pointer to
determine whether or not the root hub is registered.  The is not a
valid test, because the pointer is set before the root hub gets
registered and remains set even after the root hub is unregistered and
deallocated.  As a result, usb_device_read() or usb_device_dump() can
access freed memory, causing an oops.

The patch changes the test to use the hcd->rh_registered flag, which
does get set and cleared at the appropriate times.  It also makes sure
to hold the usb_bus_list_lock mutex while setting the flag, so that
usb_device_read() will become aware of new root hubs as soon as they
are registered.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Reported-by: Don Zickus <dzickus@redhat.com>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/usb/core/devices.c
drivers/usb/core/hcd.c

index f4ead1296820c8e789903f428a5b1d1a28bbcc6d..f460de31aceee80200ff9e91f514917bae9f7adc 100644 (file)
@@ -623,7 +623,7 @@ static ssize_t usb_device_read(struct file *file, char __user *buf,
        /* print devices for all busses */
        list_for_each_entry(bus, &usb_bus_list, bus_list) {
                /* recurse through all children of the root hub */
-               if (!bus->root_hub)
+               if (!bus_to_hcd(bus)->rh_registered)
                        continue;
                usb_lock_device(bus->root_hub);
                ret = usb_device_dump(&buf, &nbytes, &skip_bytes, ppos,
index 35b52f6e1c5e03cc9b580986f126f395082c2873..1e741bca02652e8f4ed2b027654abc3ac9d16a4f 100644 (file)
@@ -1011,10 +1011,7 @@ static int register_root_hub(struct usb_hcd *hcd)
        if (retval) {
                dev_err (parent_dev, "can't register root hub for %s, %d\n",
                                dev_name(&usb_dev->dev), retval);
-       }
-       mutex_unlock(&usb_bus_list_lock);
-
-       if (retval == 0) {
+       } else {
                spin_lock_irq (&hcd_root_hub_lock);
                hcd->rh_registered = 1;
                spin_unlock_irq (&hcd_root_hub_lock);
@@ -1023,6 +1020,7 @@ static int register_root_hub(struct usb_hcd *hcd)
                if (HCD_DEAD(hcd))
                        usb_hc_died (hcd);      /* This time clean up */
        }
+       mutex_unlock(&usb_bus_list_lock);
 
        return retval;
 }