diff options
author | Alan Stern <stern@rowland.harvard.edu> | 2007-05-22 11:46:41 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2007-07-12 19:29:48 -0400 |
commit | d4ead16f50f9ad30bdc7276ec8fee7a24c72f294 (patch) | |
tree | e1905abbc393cc4d73180dd7b9e1cf860378b590 /drivers/usb/misc/ldusb.c | |
parent | 55e5fdfa541ec7bf1b1613624ed4dd8cdacaa841 (diff) |
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>
Diffstat (limited to 'drivers/usb/misc/ldusb.c')
-rw-r--r-- | drivers/usb/misc/ldusb.c | 33 |
1 files changed, 8 insertions, 25 deletions
diff --git a/drivers/usb/misc/ldusb.c b/drivers/usb/misc/ldusb.c index 7bad49404762..5e950b90c541 100644 --- a/drivers/usb/misc/ldusb.c +++ b/drivers/usb/misc/ldusb.c | |||
@@ -176,9 +176,6 @@ struct ld_usb { | |||
176 | int interrupt_out_busy; | 176 | int interrupt_out_busy; |
177 | }; | 177 | }; |
178 | 178 | ||
179 | /* prevent races between open() and disconnect() */ | ||
180 | static DEFINE_MUTEX(disconnect_mutex); | ||
181 | |||
182 | static struct usb_driver ld_usb_driver; | 179 | static struct usb_driver ld_usb_driver; |
183 | 180 | ||
184 | /** | 181 | /** |
@@ -298,35 +295,28 @@ static int ld_usb_open(struct inode *inode, struct file *file) | |||
298 | { | 295 | { |
299 | struct ld_usb *dev; | 296 | struct ld_usb *dev; |
300 | int subminor; | 297 | int subminor; |
301 | int retval = 0; | 298 | int retval; |
302 | struct usb_interface *interface; | 299 | struct usb_interface *interface; |
303 | 300 | ||
304 | nonseekable_open(inode, file); | 301 | nonseekable_open(inode, file); |
305 | subminor = iminor(inode); | 302 | subminor = iminor(inode); |
306 | 303 | ||
307 | mutex_lock(&disconnect_mutex); | ||
308 | |||
309 | interface = usb_find_interface(&ld_usb_driver, subminor); | 304 | interface = usb_find_interface(&ld_usb_driver, subminor); |
310 | 305 | ||
311 | if (!interface) { | 306 | if (!interface) { |
312 | err("%s - error, can't find device for minor %d\n", | 307 | err("%s - error, can't find device for minor %d\n", |
313 | __FUNCTION__, subminor); | 308 | __FUNCTION__, subminor); |
314 | retval = -ENODEV; | 309 | return -ENODEV; |
315 | goto unlock_disconnect_exit; | ||
316 | } | 310 | } |
317 | 311 | ||
318 | dev = usb_get_intfdata(interface); | 312 | dev = usb_get_intfdata(interface); |
319 | 313 | ||
320 | if (!dev) { | 314 | if (!dev) |
321 | retval = -ENODEV; | 315 | return -ENODEV; |
322 | goto unlock_disconnect_exit; | ||
323 | } | ||
324 | 316 | ||
325 | /* lock this device */ | 317 | /* lock this device */ |
326 | if (down_interruptible(&dev->sem)) { | 318 | if (down_interruptible(&dev->sem)) |
327 | retval = -ERESTARTSYS; | 319 | return -ERESTARTSYS; |
328 | goto unlock_disconnect_exit; | ||
329 | } | ||
330 | 320 | ||
331 | /* allow opening only once */ | 321 | /* allow opening only once */ |
332 | if (dev->open_count) { | 322 | if (dev->open_count) { |
@@ -366,9 +356,6 @@ static int ld_usb_open(struct inode *inode, struct file *file) | |||
366 | unlock_exit: | 356 | unlock_exit: |
367 | up(&dev->sem); | 357 | up(&dev->sem); |
368 | 358 | ||
369 | unlock_disconnect_exit: | ||
370 | mutex_unlock(&disconnect_mutex); | ||
371 | |||
372 | return retval; | 359 | return retval; |
373 | } | 360 | } |
374 | 361 | ||
@@ -766,18 +753,16 @@ static void ld_usb_disconnect(struct usb_interface *intf) | |||
766 | struct ld_usb *dev; | 753 | struct ld_usb *dev; |
767 | int minor; | 754 | int minor; |
768 | 755 | ||
769 | mutex_lock(&disconnect_mutex); | ||
770 | |||
771 | dev = usb_get_intfdata(intf); | 756 | dev = usb_get_intfdata(intf); |
772 | usb_set_intfdata(intf, NULL); | 757 | usb_set_intfdata(intf, NULL); |
773 | 758 | ||
774 | down(&dev->sem); | ||
775 | |||
776 | minor = intf->minor; | 759 | minor = intf->minor; |
777 | 760 | ||
778 | /* give back our minor */ | 761 | /* give back our minor */ |
779 | usb_deregister_dev(intf, &ld_usb_class); | 762 | usb_deregister_dev(intf, &ld_usb_class); |
780 | 763 | ||
764 | down(&dev->sem); | ||
765 | |||
781 | /* if the device is not opened, then we clean up right now */ | 766 | /* if the device is not opened, then we clean up right now */ |
782 | if (!dev->open_count) { | 767 | if (!dev->open_count) { |
783 | up(&dev->sem); | 768 | up(&dev->sem); |
@@ -787,8 +772,6 @@ static void ld_usb_disconnect(struct usb_interface *intf) | |||
787 | up(&dev->sem); | 772 | up(&dev->sem); |
788 | } | 773 | } |
789 | 774 | ||
790 | mutex_unlock(&disconnect_mutex); | ||
791 | |||
792 | dev_info(&intf->dev, "LD USB Device #%d now disconnected\n", | 775 | dev_info(&intf->dev, "LD USB Device #%d now disconnected\n", |
793 | (minor - USB_LD_MINOR_BASE)); | 776 | (minor - USB_LD_MINOR_BASE)); |
794 | } | 777 | } |