- 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
This commit is contained in:
alfred 2009-08-24 05:05:38 +00:00
parent d1d25e4acf
commit 52d0adc8f6
8 changed files with 130 additions and 62 deletions

View File

@ -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 */

View File

@ -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 @@ usbd_set_config_index(struct usb_device *udev, uint8_t index)
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 @@ usb_alloc_device(device_t parent_dev, struct usb_bus *bus,
}
} 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));
}

View File

@ -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_ */

View File

@ -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 @@ usb_handle_iface_request(struct usb_xfer *xfer,
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);
}

View File

@ -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 @@ uhub_reattach_port(struct uhub_softc *sc, uint8_t portno)
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 @@ uhub_attach(device_t dev)
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 @@ usb_dev_suspend_peer(struct usb_device *udev)
}
}
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 */

View File

@ -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);
}
}

View File

@ -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_ */

View File

@ -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 */