]> git.hungrycats.org Git - linux/commitdiff
[PATCH] USB: lots of locking and other SMP race fixes
authorOliver Neukum <oliver@neukum.name>
Thu, 18 Jul 2002 08:42:49 +0000 (01:42 -0700)
committerGreg Kroah-Hartman <greg@kroah.com>
Thu, 18 Jul 2002 08:42:49 +0000 (01:42 -0700)
This is a merge of a bunch of SMP and locking fixes for the USB code
that Oliver has sent me (greg k-h) over the past few weeks.

drivers/usb/class/printer.c
drivers/usb/core/devices.c
drivers/usb/core/devio.c
drivers/usb/core/drivers.c
drivers/usb/core/file.c
drivers/usb/core/hub.c
drivers/usb/core/usb.c
drivers/usb/media/pwc-if.c
include/linux/usb.h

index cd9b52896fa5effb8bb0f984b0a442afa5dcabeb..02cd584bdaf7978c22e948e55efa12422b9ca182 100644 (file)
@@ -655,7 +655,10 @@ static ssize_t usblp_write(struct file *file, const char *buffer, size_t count,
                                                          (count - writecount) : USBLP_BUF_SIZE;
 
                if (copy_from_user(usblp->writeurb->transfer_buffer, buffer + writecount,
-                               usblp->writeurb->transfer_buffer_length)) return -EFAULT;
+                               usblp->writeurb->transfer_buffer_length)) {
+                       up(&usblp->sem);
+                       return writecount ? writecount : -EFAULT;
+               }
 
                usblp->writeurb->dev = usblp->dev;
                usblp->wcomplete = 0;
index c15f5f4e60b26102bf0a8c5cf07133b7deaf4955..e992e454f7f006084937f223ddf094bbbde4ca00 100644 (file)
@@ -152,8 +152,8 @@ static const struct class_info clas_info[] =
 
 void usbdevfs_conn_disc_event(void)
 {
-       wake_up(&deviceconndiscwq);
        conndiscevcnt++;
+       wake_up(&deviceconndiscwq);
 }
 
 static const char *class_decode(const int class)
@@ -239,6 +239,7 @@ static char *usb_dump_interface_descriptor(char *start, char *end, const struct
 
        if (start > end)
                return start;
+       lock_kernel(); /* driver might be unloaded */
        start += sprintf(start, format_iface,
                         desc->bInterfaceNumber,
                         desc->bAlternateSetting,
@@ -248,6 +249,7 @@ static char *usb_dump_interface_descriptor(char *start, char *end, const struct
                         desc->bInterfaceSubClass,
                         desc->bInterfaceProtocol,
                         iface->driver ? iface->driver->name : "(none)");
+       unlock_kernel();
        return start;
 }
 
@@ -597,6 +599,13 @@ static unsigned int usb_device_poll(struct file *file, struct poll_table_struct
                        unlock_kernel();
                        return POLLIN;
                }
+               
+               /* we may have dropped BKL - need to check for having lost the race */
+               if (file->private_data) {
+                       kfree(st);
+                       goto lost_race;
+               }
+
                /*
                 * need to prevent the module from being unloaded, since
                 * proc_unregister does not call the release method and
@@ -606,6 +615,7 @@ static unsigned int usb_device_poll(struct file *file, struct poll_table_struct
                file->private_data = st;
                mask = POLLIN;
        }
+lost_race:
        if (file->f_mode & FMODE_READ)
                 poll_wait(file, &deviceconndiscwq, wait);
        if (st->lastev != conndiscevcnt)
index b7c95b292a4842f12a08e17e2e0acb81160fb57b..bf9ef71c374a2dc76be652f5ba3caadc053d2b32 100644 (file)
@@ -361,14 +361,14 @@ static int releaseintf(struct dev_state *ps, unsigned int intf)
        if (intf >= 8*sizeof(ps->ifclaimed))
                return -EINVAL;
        err = -EINVAL;
-       lock_kernel();
        dev = ps->dev;
+       down(&dev->serialize);
        if (dev && test_and_clear_bit(intf, &ps->ifclaimed)) {
                iface = &dev->actconfig->interface[intf];
                usb_driver_release_interface(&usbdevfs_driver, iface);
                err = 0;
        }
-       unlock_kernel();
+       up(&dev->serialize);
        return err;
 }
 
@@ -722,14 +722,11 @@ static int proc_resetdevice(struct dev_state *ps)
                if (test_bit(i, &ps->ifclaimed))
                        continue;
 
-               if (intf->driver) {
-                       const struct usb_device_id *id;
-                       down(&intf->driver->serialize);
-                       intf->driver->disconnect(ps->dev, intf->private_data);
-                       id = usb_match_id(ps->dev,intf,intf->driver->id_table);
-                       intf->driver->probe(ps->dev, i, id);
-                       up(&intf->driver->serialize);
+               lock_kernel();
+               if (intf->driver && ps->dev) {
+                       usb_bind_driver(intf->driver,ps->dev, i);
                }
+               unlock_kernel();
        }
 
        return 0;
@@ -1092,16 +1089,17 @@ static int proc_ioctl (struct dev_state *ps, void *arg)
 
        /* disconnect kernel driver from interface, leaving it unbound.  */
        case USBDEVFS_DISCONNECT:
+               /* this function is voodoo. without locking it is a maybe thing */
+               lock_kernel();
                driver = ifp->driver;
                if (driver) {
-                       down (&driver->serialize);
                        dbg ("disconnect '%s' from dev %d interface %d",
                                driver->name, ps->dev->devnum, ctrl.ifno);
-                       driver->disconnect (ps->dev, ifp->private_data);
+                      usb_unbind_driver(ps->dev, ifp);
                        usb_driver_release_interface (driver, ifp);
-                       up (&driver->serialize);
                } else
                        retval = -EINVAL;
+               unlock_kernel();
                break;
 
        /* let kernel drivers try to (re)bind to the interface */
@@ -1111,18 +1109,28 @@ static int proc_ioctl (struct dev_state *ps, void *arg)
 
        /* talk directly to the interface's driver */
        default:
+               lock_kernel(); /* against module unload */
                driver = ifp->driver;
-               if (driver == 0 || driver->ioctl == 0)
-                       retval = -ENOSYS;
-               else {
-                       if (ifp->driver->owner)
+               if (driver == 0 || driver->ioctl == 0) {
+                       unlock_kernel();
+                       retval = -ENOSYS;
+               } else {
+                       if (ifp->driver->owner) {
                                __MOD_INC_USE_COUNT(ifp->driver->owner);
+                               unlock_kernel();
+                       }
                        /* ifno might usefully be passed ... */
                        retval = driver->ioctl (ps->dev, ctrl.ioctl_code, buf);
                        /* size = min_t(int, size, retval)? */
-                       if (ifp->driver->owner)
+                       if (ifp->driver->owner) {
                                __MOD_DEC_USE_COUNT(ifp->driver->owner);
+                       } else {
+                               unlock_kernel();
+                       }
                }
+               
+               if (retval == -ENOIOCTLCMD)
+                       retval = -ENOTTY;
        }
 
        /* cleanup and return */
@@ -1139,7 +1147,7 @@ static int proc_ioctl (struct dev_state *ps, void *arg)
 static int usbdev_ioctl(struct inode *inode, struct file *file, unsigned int cmd, unsigned long arg)
 {
        struct dev_state *ps = (struct dev_state *)file->private_data;
-       int ret = -ENOIOCTLCMD;
+       int ret = -ENOTTY;
 
        if (!(file->f_mode & FMODE_WRITE))
                return -EPERM;
index 3419792c7fb69bfa5b008bb3a3c056ea1045f51c..a07b4cd0acd2373830cc3679682d4fae61119982 100644 (file)
@@ -66,6 +66,7 @@ static ssize_t usb_driver_read(struct file *file, char *buf, size_t nbytes, loff
        start = page;
        end = page + (PAGE_SIZE - 100);
        pos = *ppos;
+       lock_kernel(); /* else drivers might be unloaded */
        for (; tmp != &usb_driver_list; tmp = tmp->next) {
                struct usb_driver *driver = list_entry(tmp, struct usb_driver, driver_list);
                int minor = driver->fops ? driver->minor : -1;
@@ -80,6 +81,7 @@ static ssize_t usb_driver_read(struct file *file, char *buf, size_t nbytes, loff
                        break;
                }
        }
+       unlock_kernel();
        if (start == page)
                start += sprintf(start, "(none)\n");
        len = start - page;
index eba43f1d84e5d78b1614b944e7afe09e4c73eb6e..d61ffbdf88d718f7a384816548da8a0bfa9b946b 100644 (file)
@@ -44,10 +44,13 @@ static int usb_open(struct inode * inode, struct file * file)
 
        spin_lock (&minor_lock);
        c = usb_minors[minor];
-       spin_unlock (&minor_lock);
 
-       if (!c || !(new_fops = fops_get(c)))
+       if (!c || !(new_fops = fops_get(c))) {
+               spin_unlock(&minor_lock);
                return err;
+       }
+       spin_unlock(&minor_lock);
+
        old_fops = file->f_op;
        file->f_op = new_fops;
        /* Curiouser and curiouser... NULL ->open() as "no device" ? */
index ad1422fabdfa6a946eb51e8b8bd8cc6f78cc2926..d6e152b71c2574801d87627f35575904013e2087 100644 (file)
@@ -1046,8 +1046,6 @@ static void usb_hub_events(void)
 
 static int usb_hub_thread(void *__hub)
 {
-       lock_kernel();
-
        /*
         * This thread doesn't need any user-level access,
         * so get rid of all our resources
@@ -1067,8 +1065,6 @@ static int usb_hub_thread(void *__hub)
        } while (!signal_pending(current));
 
        dbg("usb_hub_thread exiting");
-
-       unlock_kernel();
        complete_and_exit(&khubd_exited, 0);
 }
 
index 55c9e8955d24361805c53434fd5df51cb89c45d8..c7a03393e32ce010432f72fa83c67d08dae8e3e0 100644 (file)
@@ -32,6 +32,7 @@
 #include <linux/init.h>
 #include <linux/spinlock.h>
 #include <linux/errno.h>
+#include <linux/smp_lock.h>
 
 #ifdef CONFIG_USB_DEBUG
        #define DEBUG
@@ -117,6 +118,108 @@ void usb_scan_devices(void)
        up (&usb_bus_list_lock);
 }
 
+/**
+ *     usb_unbind_driver - disconnects a driver from a device
+ *     @device: usb device to be disconnected
+ *     @intf: interface of the device to be disconnected
+ *     Context: BKL held
+ *
+ *     Handles module usage count correctly
+ */
+
+void usb_unbind_driver(struct usb_device *device, struct usb_interface *intf)
+{
+       struct usb_driver *driver;
+       void *priv;
+       int m;
+       
+
+       driver = intf->driver;
+       priv = intf->private_data;
+       
+       if (!driver)
+               return;
+
+       /* as soon as we increase the module use count we drop the BKL
+          before that we must not sleep */
+       if (driver->owner) {
+               m = try_inc_mod_count(driver->owner);
+               if (m == 0) {
+                       err("Dieing driver still bound to device.\n");
+                       return;
+               }
+               unlock_kernel();
+       }
+       down(&driver->serialize);       /* if we sleep here on an umanaged driver
+                                          the holder of the lock guards against
+                                          module unload */
+
+       driver->disconnect(device, priv);
+
+       up(&driver->serialize);
+       if (driver->owner) {
+               lock_kernel();
+               __MOD_DEC_USE_COUNT(driver->owner);
+       }
+}
+
+/**
+ *     usb_bind_driver - connect a driver to a device's interface
+ *     @driver: device driver to be bound to a devices interface
+ *     @dev: device to be bound
+ *     @ifnum: index number of the interface to be used
+ *
+ *     Does a save binding of a driver to a device's interface
+ *     Returns a pointer to the drivers private description of the binding
+ */
+
+void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int ifnum)
+{
+       int i,m;
+       void *private = NULL;
+       const struct usb_device_id *id;
+       struct usb_interface *interface;
+
+       if (driver->owner) {
+               m = try_inc_mod_count(driver->owner);
+               if (m == 0)
+                       return NULL; /* this horse is dead - don't ride*/
+               unlock_kernel();
+       }
+
+       interface = &dev->actconfig->interface[ifnum];
+
+       id = driver->id_table;
+       /* new style driver? */
+       if (id) {
+               for (i = 0; i < interface->num_altsetting; i++) {
+                       interface->act_altsetting = i;
+                       id = usb_match_id(dev, interface, id);
+                       if (id) {
+                               down(&driver->serialize);
+                               private = driver->probe(dev,ifnum,id);
+                               up(&driver->serialize);
+                               if (private != NULL)
+                                       break;
+                       }
+               }
+
+               /* if driver not bound, leave defaults unchanged */
+               if (private == NULL)
+                       interface->act_altsetting = 0;
+       } else { /* "old style" driver */
+               down(&driver->serialize);
+               private = driver->probe(dev, ifnum, NULL);
+               up(&driver->serialize);
+       }
+       if (driver->owner) {
+               lock_kernel();
+               __MOD_DEC_USE_COUNT(driver->owner);
+       }
+
+       return private;
+}
+
 /*
  * This function is part of a depth-first search down the device tree,
  * removing any instances of a device driver.
@@ -136,18 +239,12 @@ static void usb_drivers_purge(struct usb_driver *driver,struct usb_device *dev)
 
        if (!dev->actconfig)
                return;
-                       
+
        for (i = 0; i < dev->actconfig->bNumInterfaces; i++) {
                struct usb_interface *interface = &dev->actconfig->interface[i];
-               
+
                if (interface->driver == driver) {
-                       if (driver->owner)
-                               __MOD_INC_USE_COUNT(driver->owner);
-                       down(&driver->serialize);
-                       driver->disconnect(dev, interface->private_data);
-                       up(&driver->serialize);
-                       if (driver->owner)
-                               __MOD_DEC_USE_COUNT(driver->owner);
+                       usb_unbind_driver(dev, interface);
                        /* if driver->disconnect didn't release the interface */
                        if (interface->driver)
                                usb_driver_release_interface(driver, interface);
@@ -163,7 +260,7 @@ static void usb_drivers_purge(struct usb_driver *driver,struct usb_device *dev)
 /**
  * usb_deregister - unregister a USB driver
  * @driver: USB operations of the driver to unregister
- * Context: !in_interrupt ()
+ * Context: !in_interrupt (), must be called with BKL held
  *
  * Unlinks the specified driver from the internal USB driver list.
  * 
@@ -528,9 +625,7 @@ static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
        struct list_head *tmp;
        struct usb_interface *interface;
        void *private;
-       const struct usb_device_id *id;
        struct usb_driver *driver;
-       int i;
        
        if ((!dev) || (ifnum >= dev->actconfig->bNumInterfaces)) {
                err("bad find_interface_driver params");
@@ -545,37 +640,12 @@ static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
                goto out_err;
 
        private = NULL;
+       lock_kernel();
        for (tmp = usb_driver_list.next; tmp != &usb_driver_list;) {
                driver = list_entry(tmp, struct usb_driver, driver_list);
                tmp = tmp->next;
 
-               if (driver->owner)
-                       __MOD_INC_USE_COUNT(driver->owner);
-               id = driver->id_table;
-               /* new style driver? */
-               if (id) {
-                       for (i = 0; i < interface->num_altsetting; i++) {
-                               interface->act_altsetting = i;
-                               id = usb_match_id(dev, interface, id);
-                               if (id) {
-                                       down(&driver->serialize);
-                                       private = driver->probe(dev,ifnum,id);
-                                       up(&driver->serialize);
-                                       if (private != NULL)
-                                               break;
-                               }
-                       }
-
-                       /* if driver not bound, leave defaults unchanged */
-                       if (private == NULL)
-                               interface->act_altsetting = 0;
-               } else { /* "old style" driver */
-                       down(&driver->serialize);
-                       private = driver->probe(dev, ifnum, NULL);
-                       up(&driver->serialize);
-               }
-               if (driver->owner)
-                       __MOD_DEC_USE_COUNT(driver->owner);
+               private = usb_bind_driver(driver, dev, ifnum);
 
                /* probe() may have changed the config on us */
                interface = dev->actconfig->interface + ifnum;
@@ -583,9 +653,11 @@ static int usb_find_interface_driver(struct usb_device *dev, unsigned ifnum)
                if (private) {
                        usb_driver_claim_interface(driver, interface, private);
                        up(&dev->serialize);
+                       unlock_kernel();
                        return 0;
                }
        }
+       unlock_kernel();
 
 out_err:
        up(&dev->serialize);
@@ -1121,27 +1193,22 @@ void usb_disconnect(struct usb_device **pdev)
 
        info("USB disconnect on device %d", dev->devnum);
 
+       lock_kernel();
        if (dev->actconfig) {
                for (i = 0; i < dev->actconfig->bNumInterfaces; i++) {
                        struct usb_interface *interface = &dev->actconfig->interface[i];
                        struct usb_driver *driver = interface->driver;
                        if (driver) {
-                               if (driver->owner)
-                                       __MOD_INC_USE_COUNT(driver->owner);
-                               down(&driver->serialize);
-                               driver->disconnect(dev, interface->private_data);
-                               up(&driver->serialize);
+                               usb_unbind_driver(dev, interface);
                                /* if driver->disconnect didn't release the interface */
                                if (interface->driver)
                                        usb_driver_release_interface(driver, interface);
-                               /* we don't need the driver any longer */
-                               if (driver->owner)
-                                       __MOD_DEC_USE_COUNT(driver->owner);
                        }
                        /* remove our device node for this interface */
                        put_device(&interface->dev);
                }
        }
+       unlock_kernel();
 
        /* Free up all the children.. */
        for (i = 0; i < USB_MAXCHILDREN; i++) {
@@ -1475,6 +1542,8 @@ EXPORT_SYMBOL(usb_new_device);
 EXPORT_SYMBOL(usb_reset_device);
 EXPORT_SYMBOL(usb_connect);
 EXPORT_SYMBOL(usb_disconnect);
+EXPORT_SYMBOL(usb_bind_driver);
+EXPORT_SYMBOL(usb_unbind_driver);
 
 EXPORT_SYMBOL(__usb_get_extra_descriptor);
 
index 7f49d02d731681d8ef2524d9bcb273c97834bf26..b09721e005266e0686ec4b9946d904737c931ecd 100644 (file)
@@ -1756,40 +1756,40 @@ static void usb_pwc_disconnect(struct usb_device *udev, void *ptr)
        pdev = (struct pwc_device *)ptr;
        if (pdev == NULL) {
                Err("pwc_disconnect() Called without private pointer.\n");
-               return;
+               goto out_err;
        }
        if (pdev->udev == NULL) {
                Err("pwc_disconnect() already called for %p\n", pdev);
-               return;
+               goto out_err;
        }
        if (pdev->udev != udev) {
                Err("pwc_disconnect() Woops: pointer mismatch udev/pdev.\n");
-               return;
+               goto out_err;
        }
 #ifdef PWC_MAGIC       
        if (pdev->magic != PWC_MAGIC) {
                Err("pwc_disconnect() Magic number failed. Consult your scrolls and try again.\n");
-               return;
+               goto out_err;
        }
-#endif 
-       
+#endif
+
        pdev->unplugged = 1;
        if (pdev->vdev != NULL) {
-               video_unregister_device(pdev->vdev); 
+               video_unregister_device(pdev->vdev);
                if (pdev->vopen) {
                        Info("Disconnected while device/video is open!\n");
-                       
+
                        /* Wake up any processes that might be waiting for
                           a frame, let them return an error condition
                         */
                        wake_up(&pdev->frameq);
-                       
+
                        /* Wait until we get a 'go' from _close(). This used
                           to have a gigantic race condition, since we kfree()
-                          stuff here, but we have to wait until close() 
-                          is finished. 
+                          stuff here, but we have to wait until close()
+                          is finished.
                         */
-                          
+
                        Trace(TRACE_PROBE, "Sleeping on remove_ok.\n");
                        add_wait_queue(&pdev->remove_ok, &wait);
                        set_current_state(TASK_UNINTERRUPTIBLE);
@@ -1815,6 +1815,7 @@ static void usb_pwc_disconnect(struct usb_device *udev, void *ptr)
                        device_hint[hint].pdev = NULL;
 
        pdev->udev = NULL;
+out_err:
        unlock_kernel();
        kfree(pdev);
 }
index 11836df8a6abe4aae2ea07f2615cd8de68628845..92496658baf5d0ceccc53826536a2e2d7a1a6e0c 100644 (file)
@@ -431,6 +431,10 @@ extern void usb_free_dev(struct usb_device *);
 /* for when layers above USB add new non-USB drivers */
 extern void usb_scan_devices(void);
 
+/* for probe/disconnect with correct module usage counting */
+void *usb_bind_driver(struct usb_driver *driver, struct usb_device *dev, unsigned int ifnum);
+void usb_unbind_driver(struct usb_device *device, struct usb_interface *intf);
+
 /* mostly for devices emulating SCSI over USB */
 extern int usb_reset_device(struct usb_device *dev);