Author: hselasky
Date: Fri Jan 24 08:06:14 2014
New Revision: 261108
URL: http://svnweb.freebsd.org/changeset/base/261108

Log:
  MFC r260808 and r260814:
  - Close a minor deadlock.
  - Fix a possible memory use after free and leak situation associated
  with USB device detach when using character device handles. This also
  includes LibUSB. It turns out that "usb_close()" cannot always get a
  reference to clean up its USB transfers and such, if called during the
  kernel USB device detach.

Modified:
  stable/8/sys/dev/usb/usb_dev.c
  stable/8/sys/dev/usb/usb_device.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/dev/   (props changed)
  stable/8/sys/dev/usb/   (props changed)

Modified: stable/8/sys/dev/usb/usb_dev.c
==============================================================================
--- stable/8/sys/dev/usb/usb_dev.c      Fri Jan 24 08:01:42 2014        
(r261107)
+++ stable/8/sys/dev/usb/usb_dev.c      Fri Jan 24 08:06:14 2014        
(r261108)
@@ -203,6 +203,11 @@ usb_ref_device(struct usb_cdev_privdata 
                DPRINTFN(2, "no device at %u\n", cpd->dev_index);
                goto error;
        }
+       if (cpd->udev->state == USB_STATE_DETACHED &&
+           (need_uref != 2)) {
+               DPRINTFN(2, "device is detached\n");
+               goto error;
+       }
        if (cpd->udev->refcount == USB_DEV_REF_MAX) {
                DPRINTFN(2, "no dev ref\n");
                goto error;
@@ -593,6 +598,13 @@ usb_fifo_free(struct usb_fifo *f)
                mtx_unlock(f->priv_mtx);
                mtx_lock(&usb_ref_lock);
 
+               /*
+                * Check if the "f->refcount" variable reached zero
+                * during the unlocked time before entering wait:
+                */
+               if (f->refcount == 0)
+                       break;
+
                /* wait for sync */
                cv_wait(&f->cv_drain, &usb_ref_lock);
        }
@@ -911,23 +923,12 @@ usb_close(void *arg)
 
        DPRINTFN(2, "cpd=%p\n", cpd);
 
-       err = usb_ref_device(cpd, &refs, 0);
-       if (err)
+       err = usb_ref_device(cpd, &refs,
+           2 /* uref and allow detached state */);
+       if (err) {
+               DPRINTFN(0, "Cannot grab USB reference when "
+                   "closing USB file handle\n");
                goto done;
-
-       /*
-        * If this function is not called directly from the root HUB
-        * thread, there is usually a need to lock the enumeration
-        * lock. Check this.
-        */
-       if (!usbd_enum_is_locked(cpd->udev)) {
-
-               DPRINTFN(2, "Locking enumeration\n");
-
-               /* reference device */
-               err = usb_usb_ref_device(cpd, &refs);
-               if (err)
-                       goto done;
        }
        if (cpd->fflags & FREAD) {
                usb_fifo_close(refs.rxfifo, cpd->fflags);

Modified: stable/8/sys/dev/usb/usb_device.c
==============================================================================
--- stable/8/sys/dev/usb/usb_device.c   Fri Jan 24 08:01:42 2014        
(r261107)
+++ stable/8/sys/dev/usb/usb_device.c   Fri Jan 24 08:06:14 2014        
(r261108)
@@ -2036,6 +2036,8 @@ usb_free_device(struct usb_device *udev,
        DPRINTFN(4, "udev=%p port=%d\n", udev, udev->port_no);
 
        bus = udev->bus;
+
+       /* set DETACHED state to prevent any further references */
        usb_set_device_state(udev, USB_STATE_DETACHED);
 
 #if USB_HAVE_DEVCTL
@@ -2051,16 +2053,7 @@ usb_free_device(struct usb_device *udev,
                usb_free_symlink(udev->ugen_symlink);
                udev->ugen_symlink = NULL;
        }
-#endif
-       /*
-        * Unregister our device first which will prevent any further
-        * references:
-        */
-       usb_bus_port_set_device(bus, udev->parent_hub ?
-           udev->parent_hub->hub->ports + udev->port_index : NULL,
-           NULL, USB_ROOT_HUB_ADDR);
 
-#if USB_HAVE_UGEN
        /* wait for all pending references to go away: */
        mtx_lock(&usb_ref_lock);
        udev->refcount--;
@@ -2080,6 +2073,11 @@ usb_free_device(struct usb_device *udev,
        /* the following will get the device unconfigured in software */
        usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_EP0);
 
+       /* final device unregister after all character devices are closed */
+       usb_bus_port_set_device(bus, udev->parent_hub ?
+           udev->parent_hub->hub->ports + udev->port_index : NULL,
+           NULL, USB_ROOT_HUB_ADDR);
+
        /* unsetup any leftover default USB transfers */
        usbd_transfer_unsetup(udev->ctrl_xfer, USB_CTRL_XFER_MAX);
 
@@ -2688,8 +2686,14 @@ usb_set_device_state(struct usb_device *
 
        DPRINTF("udev %p state %s -> %s\n", udev,
            usb_statestr(udev->state), usb_statestr(state));
-       udev->state = state;
 
+#if USB_HAVE_UGEN
+       mtx_lock(&usb_ref_lock);
+#endif
+       udev->state = state;
+#if USB_HAVE_UGEN
+       mtx_unlock(&usb_ref_lock);
+#endif
        if (udev->bus->methods->device_state_change != NULL)
                (udev->bus->methods->device_state_change) (udev);
 }
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to