USB: prevent char device open/deregister race

This patch (as908) adds central protection in usbcore for the
prototypical race between opening and unregistering a char device.
The spinlock used to protect the minor-numbers array is replaced with
an rwsem, which can remain locked across a call to a driver's open()
method.  This guarantees that open() and deregister() will be mutually
exclusive.

The private locks currently used in several individual drivers for
this purpose are no longer necessary, and the patch removes them.  The
following USB drivers are affected: usblcd, idmouse, auerswald,
legousbtower, sisusbvga/sisusb, ldusb, adutux, iowarrior, and
usb-skeleton.

As a side effect of this change, usb_deregister_dev() must not be
called while holding a lock that is acquired by open().  Unfortunately
a number of drivers do this, but luckily the solution is simple: call
usb_deregister_dev() before acquiring the lock.

In addition to these changes (and their consequent code
simplifications), the patch fixes a use-after-free bug in adutux and a
race between open() and release() in iowarrior.

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 6 insertions, 18 deletions

diff --git a/drivers/usb/misc/legousbtower.c b/drivers/usb/misc/legousbtower.c
index 1713e19a789..2ed0daea894 100644
--- a/drivers/usb/misc/legousbtower.c
+++ b/drivers/usb/misc/legousbtower.c
@@ -254,9 +254,6 @@ static int  tower_probe	(struct usb_interface *interface, const struct usb_devic
 static void tower_disconnect	(struct usb_interface *interface);
 
 
-/* prevent races between open() and disconnect */
-static DEFINE_MUTEX (disconnect_mutex);
-
 /* file operations needed when we register this driver */
 static const struct file_operations tower_fops = {
 	.owner =	THIS_MODULE,
@@ -344,28 +341,26 @@ static int tower_open (struct inode *inode, struct file *file)
 	nonseekable_open(inode, file);
 	subminor = iminor(inode);
 
-	mutex_lock (&disconnect_mutex);
-
 	interface = usb_find_interface (&tower_driver, subminor);
 
 	if (!interface) {
 		err ("%s - error, can't find device for minor %d",
 		     __FUNCTION__, subminor);
 		retval = -ENODEV;
-		goto unlock_disconnect_exit;
+		goto exit;
 	}
 
 	dev = usb_get_intfdata(interface);
 
 	if (!dev) {
 		retval = -ENODEV;
-		goto unlock_disconnect_exit;
+		goto exit;
 	}
 
 	/* lock this device */
 	if (down_interruptible (&dev->sem)) {
 	        retval = -ERESTARTSYS;
-		goto unlock_disconnect_exit;
+		goto exit;
 	}
 
 	/* allow opening only once */
@@ -421,9 +416,7 @@ static int tower_open (struct inode *inode, struct file *file)
 unlock_exit:
 	up (&dev->sem);
 
-unlock_disconnect_exit:
-	mutex_unlock (&disconnect_mutex);
-
+exit:
 	dbg(2, "%s: leave, return value %d ", __FUNCTION__, retval);
 
 	return retval;
@@ -993,19 +986,16 @@ static void tower_disconnect (struct usb_interface *interface)
 
 	dbg(2, "%s: enter", __FUNCTION__);
 
-	mutex_lock (&disconnect_mutex);
-
 	dev = usb_get_intfdata (interface);
 	usb_set_intfdata (interface, NULL);
 
-
-	down (&dev->sem);
-
 	minor = dev->minor;
 
 	/* give back our minor */
 	usb_deregister_dev (interface, &tower_class);
 
+	down (&dev->sem);
+
 	/* if the device is not opened, then we clean up right now */
 	if (!dev->open_count) {
 		up (&dev->sem);
@@ -1015,8 +1005,6 @@ static void tower_disconnect (struct usb_interface *interface)
 		up (&dev->sem);
 	}
 
-	mutex_unlock (&disconnect_mutex);
-
 	info("LEGO USB Tower #%d now disconnected", (minor - LEGO_USB_TOWER_MINOR_BASE));
 
 	dbg(2, "%s: leave", __FUNCTION__);