Try to resolve a possible deadlock when detaching USB devices which

create character devices. The deadlock can happen if an application is
issuing IOCTLs which require USB refcounting, at the same time the USB
device is detaching.

There is already a counter in place in the USB device structure to
detect this situation, but it was not always checked ahead of invoking
functions that might destroy character devices, like detach, set
configuration, set alternate interface or detach active kernel driver.

Reported by:	Daniel O'Connor <doconnor@gsoft.com.au>
MFC after:	1 week

4 files changed, 92 insertions, 18 deletions

diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c
index c81b942..e4471ae 100644
--- a/sys/dev/usb/usb_dev.c
+++ b/sys/dev/usb/usb_dev.c
@@ -214,12 +214,13 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
 		DPRINTFN(2, "device is detached\n");
 		goto error;
 	}
-	if (cpd->udev->refcount == USB_DEV_REF_MAX) {
-		DPRINTFN(2, "no dev ref\n");
-		goto error;
-	}
 	if (need_uref) {
 		DPRINTFN(2, "ref udev - needed\n");
+
+		if (cpd->udev->refcount == USB_DEV_REF_MAX) {
+			DPRINTFN(2, "no dev ref\n");
+			goto error;
+		}
 		cpd->udev->refcount++;
 
 		mtx_unlock(&usb_ref_lock);
@@ -293,9 +294,8 @@ error:
 		usbd_enum_unlock(cpd->udev);
 
 	if (crd->is_uref) {
-		if (--(cpd->udev->refcount) == 0) {
-			cv_signal(&cpd->udev->ref_cv);
-		}
+		cpd->udev->refcount--;
+		cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 	DPRINTFN(2, "fail\n");
@@ -361,10 +361,9 @@ usb_unref_device(struct usb_cdev_privdata *cpd,
 		crd->is_write = 0;
 	}
 	if (crd->is_uref) {
-		if (--(cpd->udev->refcount) == 0) {
-			cv_signal(&cpd->udev->ref_cv);
-		}
 		crd->is_uref = 0;
+		cpd->udev->refcount--;
+		cv_broadcast(&cpd->udev->ref_cv);
 	}
 	mtx_unlock(&usb_ref_lock);
 }
diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c
index 0e9176c..8f7b312 100644
--- a/sys/dev/usb/usb_device.c
+++ b/sys/dev/usb/usb_device.c
@@ -452,6 +452,65 @@ usb_endpoint_foreach(struct usb_device *udev, struct usb_endpoint *ep)
 }
 
 /*------------------------------------------------------------------------*
+ *	usb_wait_pending_ref_locked
+ *
+ * This function will wait for any USB references to go away before
+ * returning and disable further USB device refcounting on the
+ * specified USB device. This function is used when detaching a USB
+ * device.
+ *------------------------------------------------------------------------*/
+static void
+usb_wait_pending_ref_locked(struct usb_device *udev)
+{
+#if USB_HAVE_UGEN
+	const uint16_t refcount =
+	    usb_proc_is_called_from(
+	    USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2;
+
+	DPRINTF("Refcount = %d\n", (int)refcount); 
+
+	while (1) {
+		/* wait for any pending references to go away */
+		mtx_lock(&usb_ref_lock);
+		if (udev->refcount == refcount) {
+			/* prevent further refs being taken */
+			udev->refcount = USB_DEV_REF_MAX;
+			mtx_unlock(&usb_ref_lock);
+			break;
+		}
+		usbd_enum_unlock(udev);
+		cv_wait(&udev->ref_cv, &usb_ref_lock);
+		mtx_unlock(&usb_ref_lock);
+		(void) usbd_enum_lock(udev);
+	}
+#endif
+}
+
+/*------------------------------------------------------------------------*
+ *	usb_ref_restore_locked
+ *
+ * This function will restore the reference count value after a call
+ * to "usb_wait_pending_ref_locked()".
+ *------------------------------------------------------------------------*/
+static void
+usb_ref_restore_locked(struct usb_device *udev)
+{
+#if USB_HAVE_UGEN
+	const uint16_t refcount =
+	    usb_proc_is_called_from(
+	    USB_BUS_EXPLORE_PROC(udev->bus)) ? 1 : 2;
+
+	DPRINTF("Refcount = %d\n", (int)refcount); 
+
+	/* restore reference count and wakeup waiters, if any */
+	mtx_lock(&usb_ref_lock);
+	udev->refcount = refcount;
+	cv_broadcast(&udev->ref_cv);
+	mtx_unlock(&usb_ref_lock);
+#endif
+}
+
+/*------------------------------------------------------------------------*
  *	usb_unconfigure
  *
  * This function will free all USB interfaces and USB endpoints belonging
@@ -1121,6 +1180,9 @@ usb_detach_device(struct usb_device *udev, uint8_t iface_index,
 
 	sx_assert(&udev->enum_sx, SA_LOCKED);
 
+	/* wait for pending refs to go away */
+	usb_wait_pending_ref_locked(udev);
+
 	/*
 	 * First detach the child to give the child's detach routine a
 	 * chance to detach the sub-devices in the correct order.
@@ -1147,6 +1209,8 @@ usb_detach_device(struct usb_device *udev, uint8_t iface_index,
 		usb_detach_device_sub(udev, &iface->subdev,
 		    &iface->pnpinfo, flag);
 	}
+
+	usb_ref_restore_locked(udev);
 }
 
 /*------------------------------------------------------------------------*
@@ -2088,14 +2152,6 @@ usb_free_device(struct usb_device *udev, uint8_t flag)
 		udev->ugen_symlink = NULL;
 	}
 
-	/* wait for all pending references to go away: */
-	mtx_lock(&usb_ref_lock);
-	udev->refcount--;
-	while (udev->refcount != 0) {
-		cv_wait(&udev->ref_cv, &usb_ref_lock);
-	}
-	mtx_unlock(&usb_ref_lock);
-
 	usb_destroy_dev(udev->ctrl_dev);
 #endif
 
@@ -2612,8 +2668,14 @@ usb_fifo_free_wrap(struct usb_device *udev,
 			/* no need to free this FIFO */
 			continue;
 		}
+		/* wait for pending refs to go away */
+		usb_wait_pending_ref_locked(udev);
+
 		/* free this FIFO */
 		usb_fifo_free(f);
+
+		/* restore refcount */
+		usb_ref_restore_locked(udev);
 	}
 }
 #endif
diff --git a/sys/dev/usb/usb_process.c b/sys/dev/usb/usb_process.c
index 7493e39..e70166c 100644
--- a/sys/dev/usb/usb_process.c
+++ b/sys/dev/usb/usb_process.c
@@ -501,3 +501,15 @@ usb_proc_rewakeup(struct usb_process *up)
 		cv_signal(&up->up_cv);
 	}
 }
+
+/*------------------------------------------------------------------------*
+ *	usb_proc_is_called_from
+ *
+ * This function will return non-zero if called from inside the USB
+ * process passed as first argument. Else this function returns zero.
+ *------------------------------------------------------------------------*/
+int
+usb_proc_is_called_from(struct usb_process *up)
+{
+	return (up->up_curtd == curthread);
+}
diff --git a/sys/dev/usb/usb_process.h b/sys/dev/usb/usb_process.h
index 97262c4..c12cdc4 100644
--- a/sys/dev/usb/usb_process.h
+++ b/sys/dev/usb/usb_process.h
@@ -81,6 +81,7 @@ void	usb_proc_mwait(struct usb_process *up, void *pm0, void *pm1);
 void	usb_proc_free(struct usb_process *up);
 void   *usb_proc_msignal(struct usb_process *up, void *pm0, void *pm1);
 void	usb_proc_rewakeup(struct usb_process *up);
+int	usb_proc_is_called_from(struct usb_process *up);
 
 void	usb_proc_explore_mwait(struct usb_device *, void *, void *);
 void   *usb_proc_explore_msignal(struct usb_device *, void *, void *);