svn commit: r260814 - head/sys/dev/usb

Hans Petter Selasky hselasky at FreeBSD.org
Fri Jan 17 10:35:19 UTC 2014


Author: hselasky
Date: Fri Jan 17 10:35:18 2014
New Revision: 260814
URL: http://svnweb.freebsd.org/changeset/base/260814

Log:
  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.
  
  Analysis by:	hselasky @
  Reported by:	Juergen Lock <nox at jelal.kn-bremen.de>
  MFC after:	1 week

Modified:
  head/sys/dev/usb/usb_dev.c
  head/sys/dev/usb/usb_device.c

Modified: head/sys/dev/usb/usb_dev.c
==============================================================================
--- head/sys/dev/usb/usb_dev.c	Fri Jan 17 10:34:01 2014	(r260813)
+++ head/sys/dev/usb/usb_dev.c	Fri Jan 17 10:35:18 2014	(r260814)
@@ -207,6 +207,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;
@@ -922,23 +927,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: head/sys/dev/usb/usb_device.c
==============================================================================
--- head/sys/dev/usb/usb_device.c	Fri Jan 17 10:34:01 2014	(r260813)
+++ head/sys/dev/usb/usb_device.c	Fri Jan 17 10:35:18 2014	(r260814)
@@ -2070,6 +2070,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
@@ -2085,16 +2087,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--;
@@ -2114,6 +2107,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);
 
@@ -2647,8 +2645,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);
 }


More information about the svn-src-head mailing list