- Patch to allow USB controller to resume operation after

         being polled.

         - Remove the need for Giant from the USB HUB driver.

         - Leave device unconfigured instead of disabling the USB port
           when Huawei Autoinstall disk detection triggers. This should
           fix problems that the Huawei device is not detected after
           Autoinstall eject is issued.
         - Reported by: Nikolay Antsiferov

         - Fix memory use after free race for USB character devices.
         - Reported by: Lucius Windschuh

         - Factor out the enumeration lock into three functions to make the
         coming newbus lock conversion more easy.
          - usbd_enum_lock
          - usbd_enum_unlock
          - usbd_enum_is_locked

Submitted by:	hps

8 files changed, 130 insertions, 62 deletions

diff --git a/sys/dev/usb/usb_dev.c b/sys/dev/usb/usb_dev.c
index cba8919..c828b24 100644
--- a/sys/dev/usb/usb_dev.c
+++ b/sys/dev/usb/usb_dev.c
@@ -217,7 +217,7 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
 		 * We need to grab the sx-lock before grabbing the
 		 * FIFO refs to avoid deadlock at detach!
 		 */
-		sx_xlock(cpd->udev->default_sx + 1);
+		usbd_enum_lock(cpd->udev);
 
 		mtx_lock(&usb_ref_lock);
 
@@ -275,14 +275,12 @@ usb_ref_device(struct usb_cdev_privdata *cpd,
 	}
 	mtx_unlock(&usb_ref_lock);
 
-	if (crd->is_uref) {
-		mtx_lock(&Giant);	/* XXX */
-	}
 	return (0);
 
 error:
 	if (crd->is_uref) {
-		sx_unlock(cpd->udev->default_sx + 1);
+		usbd_enum_unlock(cpd->udev);
+
 		if (--(cpd->udev->refcount) == 0) {
 			cv_signal(cpd->udev->default_cv + 1);
 		}
@@ -334,10 +332,9 @@ usb_unref_device(struct usb_cdev_privdata *cpd,
 
 	DPRINTFN(2, "cpd=%p is_uref=%d\n", cpd, crd->is_uref);
 
-	if (crd->is_uref) {
-		mtx_unlock(&Giant);	/* XXX */
-		sx_unlock(cpd->udev->default_sx + 1);
-	}
+	if (crd->is_uref)
+		usbd_enum_unlock(cpd->udev);
+
 	mtx_lock(&usb_ref_lock);
 	if (crd->is_read) {
 		if (--(crd->rxfifo->refcount) == 0) {
@@ -1042,9 +1039,9 @@ usb_ioctl(struct cdev *dev, u_long cmd, caddr_t addr, int fflag, struct thread*
 	 * reference if we need it!
 	 */
 	err = usb_ref_device(cpd, &refs, 0 /* no uref */ );
-	if (err) {
+	if (err)
 		return (ENXIO);
-	}
+
 	fflags = cpd->fflags;
 
 	f = NULL;			/* set default value */
diff --git a/sys/dev/usb/usb_device.c b/sys/dev/usb/usb_device.c
index 2d34017..635a9b1 100644
--- a/sys/dev/usb/usb_device.c
+++ b/sys/dev/usb/usb_device.c
@@ -402,11 +402,11 @@ usb_unconfigure(struct usb_device *udev, uint8_t flag)
 	uint8_t do_unlock;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	/* detach all interface drivers */
@@ -442,9 +442,8 @@ usb_unconfigure(struct usb_device *udev, uint8_t flag)
 	udev->curr_config_no = USB_UNCONFIG_NO;
 	udev->curr_config_index = USB_UNCONFIG_INDEX;
 
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
 }
 
 /*------------------------------------------------------------------------*
@@ -472,11 +471,11 @@ usbd_set_config_index(struct usb_device *udev, uint8_t index)
 	DPRINTFN(6, "udev=%p index=%d\n", udev, index);
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
@@ -585,9 +584,8 @@ done:
 	if (err) {
 		usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
 	}
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
 	return (err);
 }
 
@@ -823,11 +821,11 @@ usbd_set_alt_interface_index(struct usb_device *udev,
 	uint8_t do_unlock;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 	if (iface == NULL) {
 		err = USB_ERR_INVAL;
@@ -863,9 +861,9 @@ usbd_set_alt_interface_index(struct usb_device *udev,
 	    iface->idesc->bAlternateSetting);
 
 done:
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (err);
 }
 
@@ -1230,11 +1228,11 @@ usb_probe_and_attach(struct usb_device *udev, uint8_t iface_index)
 		return (USB_ERR_INVAL);
 	}
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	if (udev->curr_config_index == USB_UNCONFIG_INDEX) {
@@ -1315,9 +1313,9 @@ usb_probe_and_attach(struct usb_device *udev, uint8_t iface_index)
 		}
 	}
 done:
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (0);
 }
 
@@ -1779,7 +1777,8 @@ repeat_set_config:
 			}
 		} else if (usb_test_huawei_autoinst_p(udev, &uaa) == 0) {
 			DPRINTFN(0, "Found Huawei auto-install disk!\n");
-			err = USB_ERR_STALLED;	/* fake an error */
+			/* leave device unconfigured */
+			usb_unconfigure(udev, USB_UNCFG_FLAG_FREE_SUBDEV);
 		}
 	} else {
 		err = 0;		/* set success */
@@ -1902,15 +1901,18 @@ static void
 usb_cdev_free(struct usb_device *udev)
 {
 	struct usb_fs_privdata* pd;
+	struct cdev* pcdev;
 
 	DPRINTFN(2, "Freeing device nodes\n");
 
 	while ((pd = LIST_FIRST(&udev->pd_list)) != NULL) {
 		KASSERT(pd->cdev->si_drv1 == pd, ("privdata corrupt"));
 
-		destroy_dev_sched_cb(pd->cdev, usb_cdev_cleanup, pd);
+		pcdev = pd->cdev;
 		pd->cdev = NULL;
 		LIST_REMOVE(pd, pd_next);
+		if (pcdev != NULL)
+			destroy_dev_sched_cb(pcdev, usb_cdev_cleanup, pd);
 	}
 }
 
@@ -2448,3 +2450,37 @@ usbd_device_attached(struct usb_device *udev)
 {
 	return (udev->state > USB_STATE_DETACHED);
 }
+
+/* The following function locks enumerating the given USB device. */
+
+void
+usbd_enum_lock(struct usb_device *udev)
+{
+	sx_xlock(udev->default_sx + 1);
+	/* 
+	 * NEWBUS LOCK NOTE: We should check if any parent SX locks
+	 * are locked before locking Giant. Else the lock can be
+	 * locked multiple times.
+	 */
+	mtx_lock(&Giant);
+}
+
+/* The following function unlocks enumerating the given USB device. */
+
+void
+usbd_enum_unlock(struct usb_device *udev)
+{
+	mtx_unlock(&Giant);
+	sx_xunlock(udev->default_sx + 1);
+}
+
+/*
+ * The following function checks the enumerating lock for the given
+ * USB device.
+ */
+
+uint8_t
+usbd_enum_is_locked(struct usb_device *udev)
+{
+	return (sx_xlocked(udev->default_sx + 1));
+}
diff --git a/sys/dev/usb/usb_device.h b/sys/dev/usb/usb_device.h
index d601c14..682e200 100644
--- a/sys/dev/usb/usb_device.h
+++ b/sys/dev/usb/usb_device.h
@@ -211,5 +211,8 @@ uint8_t	usb_peer_can_wakeup(struct usb_device *udev);
 struct usb_endpoint *usb_endpoint_foreach(struct usb_device *udev, struct usb_endpoint *ep);
 void	usb_set_device_state(struct usb_device *udev,
 	    enum usb_dev_state state);
+void	usbd_enum_lock(struct usb_device *);
+void	usbd_enum_unlock(struct usb_device *);
+uint8_t usbd_enum_is_locked(struct usb_device *);
 
 #endif					/* _USB_DEVICE_H_ */
diff --git a/sys/dev/usb/usb_handle_request.c b/sys/dev/usb/usb_handle_request.c
index a720919..92bf832 100644
--- a/sys/dev/usb/usb_handle_request.c
+++ b/sys/dev/usb/usb_handle_request.c
@@ -152,8 +152,8 @@ usb_handle_set_config(struct usb_xfer *xfer, uint8_t conf_no)
 	 * attach:
 	 */
 	USB_XFER_UNLOCK(xfer);
-	mtx_lock(&Giant);		/* XXX */
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
 
 	if (conf_no == USB_UNCONFIG_NO) {
 		conf_no = USB_UNCONFIG_INDEX;
@@ -176,8 +176,7 @@ usb_handle_set_config(struct usb_xfer *xfer, uint8_t conf_no)
 		goto done;
 	}
 done:
-	mtx_unlock(&Giant);		/* XXX */
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (err);
 }
@@ -190,19 +189,19 @@ usb_check_alt_setting(struct usb_device *udev,
 	usb_error_t err = 0;
 
 	/* automatic locking */
-	if (sx_xlocked(udev->default_sx + 1)) {
+	if (usbd_enum_is_locked(udev)) {
 		do_unlock = 0;
 	} else {
 		do_unlock = 1;
-		sx_xlock(udev->default_sx + 1);
+		usbd_enum_lock(udev);
 	}
 
 	if (alt_index >= usbd_get_no_alts(udev->cdesc, iface->idesc))
 		err = USB_ERR_INVAL;
 
-	if (do_unlock) {
-		sx_unlock(udev->default_sx + 1);
-	}
+	if (do_unlock)
+		usbd_enum_unlock(udev);
+
 	return (err);
 }
 
@@ -236,8 +235,8 @@ usb_handle_iface_request(struct usb_xfer *xfer,
 	 * attach:
 	 */
 	USB_XFER_UNLOCK(xfer);
-	mtx_lock(&Giant);		/* XXX */
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
 
 	error = ENXIO;
 
@@ -353,20 +352,17 @@ tr_repeat:
 		goto tr_stalled;
 	}
 tr_valid:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (0);
 
 tr_short:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (USB_ERR_SHORT_XFER);
 
 tr_stalled:
-	mtx_unlock(&Giant);
-	sx_unlock(udev->default_sx + 1);
+	usbd_enum_unlock(udev);
 	USB_XFER_LOCK(xfer);
 	return (USB_ERR_STALLED);
 }
diff --git a/sys/dev/usb/usb_hub.c b/sys/dev/usb/usb_hub.c
index 0defc97..2c81d97 100644
--- a/sys/dev/usb/usb_hub.c
+++ b/sys/dev/usb/usb_hub.c
@@ -96,6 +96,7 @@ struct uhub_current_state {
 struct uhub_softc {
 	struct uhub_current_state sc_st;/* current state */
 	device_t sc_dev;		/* base device */
+	struct mtx sc_mtx;		/* our mutex */
 	struct usb_device *sc_udev;	/* USB device */
 	struct usb_xfer *sc_xfer[UHUB_N_TRANSFER];	/* interrupt xfer */
 	uint8_t	sc_flags;
@@ -428,7 +429,6 @@ repeat:
 		mode = USB_MODE_HOST;
 
 	/* need to create a new child */
-
 	child = usb_alloc_device(sc->sc_dev, udev->bus, udev,
 	    udev->depth + 1, portno - 1, portno, speed, mode);
 	if (child == NULL) {
@@ -691,6 +691,8 @@ uhub_attach(device_t dev)
 	sc->sc_udev = udev;
 	sc->sc_dev = dev;
 
+	mtx_init(&sc->sc_mtx, "USB HUB mutex", NULL, MTX_DEF);
+
 	snprintf(sc->sc_name, sizeof(sc->sc_name), "%s",
 	    device_get_nameunit(dev));
 
@@ -774,7 +776,7 @@ uhub_attach(device_t dev)
 	} else {
 		/* normal HUB */
 		err = usbd_transfer_setup(udev, &iface_index, sc->sc_xfer,
-		    uhub_config, UHUB_N_TRANSFER, sc, &Giant);
+		    uhub_config, UHUB_N_TRANSFER, sc, &sc->sc_mtx);
 	}
 	if (err) {
 		DPRINTFN(0, "cannot setup interrupt transfer, "
@@ -850,9 +852,9 @@ uhub_attach(device_t dev)
 	/* Start the interrupt endpoint, if any */
 
 	if (sc->sc_xfer[0] != NULL) {
-		USB_XFER_LOCK(sc->sc_xfer[0]);
+		mtx_lock(&sc->sc_mtx);
 		usbd_transfer_start(sc->sc_xfer[0]);
-		USB_XFER_UNLOCK(sc->sc_xfer[0]);
+		mtx_unlock(&sc->sc_mtx);
 	}
 
 	/* Enable automatic power save on all USB HUBs */
@@ -868,6 +870,9 @@ error:
 		free(udev->hub, M_USBDEV);
 		udev->hub = NULL;
 	}
+
+	mtx_destroy(&sc->sc_mtx);
+
 	return (ENXIO);
 }
 
@@ -908,6 +913,9 @@ uhub_detach(device_t dev)
 
 	free(hub, M_USBDEV);
 	sc->sc_udev->hub = NULL;
+
+	mtx_destroy(&sc->sc_mtx);
+
 	return (0);
 }
 
@@ -1775,10 +1783,13 @@ usb_dev_resume_peer(struct usb_device *udev)
 		/* always update hardware power! */
 		(bus->methods->set_hw_power) (bus);
 	}
-	sx_xlock(udev->default_sx + 1);
+
+	usbd_enum_lock(udev);
+
 	/* notify all sub-devices about resume */
 	err = usb_suspend_resume(udev, 0);
-	sx_unlock(udev->default_sx + 1);
+
+	usbd_enum_unlock(udev);
 
 	/* check if peer has wakeup capability */
 	if (usb_peer_can_wakeup(udev)) {
@@ -1844,10 +1855,12 @@ repeat:
 		}
 	}
 
-	sx_xlock(udev->default_sx + 1);
+	usbd_enum_lock(udev);
+
 	/* notify all sub-devices about suspend */
 	err = usb_suspend_resume(udev, 1);
-	sx_unlock(udev->default_sx + 1);
+
+	usbd_enum_unlock(udev);
 
 	if (usb_peer_can_wakeup(udev)) {
 		/* allow device to do remote wakeup */
diff --git a/sys/dev/usb/usb_process.c b/sys/dev/usb/usb_process.c
index 5b98a81..0810885 100644
--- a/sys/dev/usb/usb_process.c
+++ b/sys/dev/usb/usb_process.c
@@ -448,3 +448,29 @@ usb_proc_drain(struct usb_process *up)
 	}
 	mtx_unlock(up->up_mtx);
 }
+
+/*------------------------------------------------------------------------*
+ *	usb_proc_rewakeup
+ *
+ * This function is called to re-wakeup the the given USB
+ * process. This usually happens after that the USB system has been in
+ * polling mode, like during a panic. This function must be called
+ * having "up->up_mtx" locked.
+ *------------------------------------------------------------------------*/
+void
+usb_proc_rewakeup(struct usb_process *up)
+{
+	/* check if not initialised */
+	if (up->up_mtx == NULL)
+		return;
+	/* check if gone */
+	if (up->up_gone)
+		return;
+
+	mtx_assert(up->up_mtx, MA_OWNED);
+
+	if (up->up_msleep == 0) {
+		/* re-wakeup */
+		cv_signal(&up->up_cv);
+	}
+}
diff --git a/sys/dev/usb/usb_process.h b/sys/dev/usb/usb_process.h
index 71432c3..b4159af 100644
--- a/sys/dev/usb/usb_process.h
+++ b/sys/dev/usb/usb_process.h
@@ -75,5 +75,6 @@ void	usb_proc_drain(struct usb_process *up);
 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);
 
 #endif					/* _USB_PROCESS_H_ */
diff --git a/sys/dev/usb/usb_transfer.c b/sys/dev/usb/usb_transfer.c
index 3c466fb..8daf475 100644
--- a/sys/dev/usb/usb_transfer.c
+++ b/sys/dev/usb/usb_transfer.c
@@ -2907,13 +2907,9 @@ usbd_transfer_poll(struct usb_xfer **ppxfer, uint16_t max)
 		}
 
 		/* Make sure cv_signal() and cv_broadcast() is not called */
-		udev->bus->control_xfer_proc.up_dsleep = 0;
 		udev->bus->control_xfer_proc.up_msleep = 0;
-		udev->bus->explore_proc.up_dsleep = 0;
 		udev->bus->explore_proc.up_msleep = 0;
-		udev->bus->giant_callback_proc.up_dsleep = 0;
 		udev->bus->giant_callback_proc.up_msleep = 0;
-		udev->bus->non_giant_callback_proc.up_dsleep = 0;
 		udev->bus->non_giant_callback_proc.up_msleep = 0;
 
 		/* poll USB hardware */