From d4ead16f50f9ad30bdc7276ec8fee7a24c72f294 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Tue, 22 May 2007 11:46:41 -0400 Subject: 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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/misc/adutux.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) (limited to 'drivers/usb/misc/adutux.c') diff --git a/drivers/usb/misc/adutux.c b/drivers/usb/misc/adutux.c index 77145f9db043..d72c42e5f22d 100644 --- a/drivers/usb/misc/adutux.c +++ b/drivers/usb/misc/adutux.c @@ -108,8 +108,6 @@ struct adu_device { struct urb* interrupt_out_urb; }; -/* prevent races between open() and disconnect */ -static DEFINE_MUTEX(disconnect_mutex); static struct usb_driver adu_driver; static void adu_debug_data(int level, const char *function, int size, @@ -256,8 +254,6 @@ static int adu_open(struct inode *inode, struct file *file) subminor = iminor(inode); - mutex_lock(&disconnect_mutex); - interface = usb_find_interface(&adu_driver, subminor); if (!interface) { err("%s - error, can't find device for minor %d", @@ -306,7 +302,6 @@ static int adu_open(struct inode *inode, struct file *file) up(&dev->sem); exit_no_device: - mutex_unlock(&disconnect_mutex); dbg(2,"%s : leave, return value %d ", __FUNCTION__, retval); return retval; @@ -318,12 +313,6 @@ static int adu_release_internal(struct adu_device *dev) dbg(2," %s : enter", __FUNCTION__); - if (dev->udev == NULL) { - /* the device was unplugged before the file was released */ - adu_delete(dev); - goto exit; - } - /* decrement our usage count for the device */ --dev->open_count; dbg(2," %s : open count %d", __FUNCTION__, dev->open_count); @@ -332,7 +321,6 @@ static int adu_release_internal(struct adu_device *dev) dev->open_count = 0; } -exit: dbg(2," %s : leave", __FUNCTION__); return retval; } @@ -367,8 +355,15 @@ static int adu_release(struct inode *inode, struct file *file) goto exit; } - /* do the work */ - retval = adu_release_internal(dev); + if (dev->udev == NULL) { + /* the device was unplugged before the file was released */ + up(&dev->sem); + adu_delete(dev); + dev = NULL; + } else { + /* do the work */ + retval = adu_release_internal(dev); + } exit: if (dev) @@ -831,19 +826,17 @@ static void adu_disconnect(struct usb_interface *interface) dbg(2," %s : enter", __FUNCTION__); - mutex_lock(&disconnect_mutex); /* not interruptible */ - dev = usb_get_intfdata(interface); usb_set_intfdata(interface, NULL); - down(&dev->sem); /* not interruptible */ - minor = dev->minor; /* give back our minor */ usb_deregister_dev(interface, &adu_class); dev->minor = 0; + down(&dev->sem); /* not interruptible */ + /* if the device is not opened, then we clean up right now */ dbg(2," %s : open count %d", __FUNCTION__, dev->open_count); if (!dev->open_count) { @@ -854,8 +847,6 @@ static void adu_disconnect(struct usb_interface *interface) up(&dev->sem); } - mutex_unlock(&disconnect_mutex); - dev_info(&interface->dev, "ADU device adutux%d now disconnected", (minor - ADU_MINOR_BASE)); -- cgit v1.2.3