ChangeSet 1.1255, 2003/06/18 16:55:30-07:00, johannes@erdfelt.com

[PATCH] USB: fix 2.4 usbdevfs race

Here's a patch to fix a race condition in usbdevfs. The fix is in hub.c
but the race is related to usbdevfs.

The race goes like this:

Process 1 (khubd)			Process 2 (mount)
usb_hub_port_connect_change()
  hub->children[port] = dev
  usb_new_device()
					usbdevfs_read_super()
					  recurse_new_dev_inode()
					    new_dev_inode()
					      list_add_tail(..., &dev->inodes)
    usbdevfs_add_device()
      new_dev_inode()
        list_add_tail(..., &dev->inodes)

The problem is that the inode gets added twice, corrupting dev->inodes.
This will cause a problems at disconnect where the same inode will be
freed twice, causing a neverending loop, or an oops. I think it will
also cause problems at unmount.

The fix is to just move setting hub->children to later in the
enumeration process. This way usbdevfs_read_super won't see the device
before it has been through the usbdevfs_add_device path.

I didn't see this on x86, but apparentely others have looking at the
RedHat 9 kernel sources. (RedHat bugzilla #81091)

Pete, could you give this patch a shot for the problem you found in that
bug? I'm pretty sure they are the same problem.


 drivers/usb/hub.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)


diff -Nru a/drivers/usb/hub.c b/drivers/usb/hub.c
--- a/drivers/usb/hub.c	Wed Jun 18 17:34:56 2003
+++ b/drivers/usb/hub.c	Wed Jun 18 17:34:56 2003
@@ -716,8 +716,6 @@
 			break;
 		}
 
-		hub->children[port] = dev;
-
 		/* Reset the device */
 		if (usb_hub_port_reset(hub, port, dev, delay)) {
 			usb_free_dev(dev);
@@ -761,8 +759,10 @@
 			dev->bus->bus_name, dev->devpath, dev->devnum);
 
 		/* Run it through the hoops (find a driver, etc) */
-		if (!usb_new_device(dev))
+		if (!usb_new_device(dev)) {
+			hub->children[port] = dev;
 			goto done;
+		}
 
 		/* Free the configuration if there was an error */
 		usb_free_dev(dev);
@@ -771,7 +771,6 @@
 		delay = HUB_LONG_RESET_TIME;
 	}
 
-	hub->children[port] = NULL;
 	usb_hub_port_disable(hub, port);
 done:
 	up(&usb_address0_sem);