From a95cd586ed6485c099f7775f56945b08d420b1b2 Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Fri, 14 Feb 2020 18:39:50 +0000 Subject: [PATCH] Fix mistakes in previous sloppy change with root intr xfers. - Make sure ux_status is set to USBD_IN_PROGRESS when started. Otherwise, if it is still in flight when we abort the pipe, usbd_ar_pipe will skip calling upm_abort. - Initialize ux_status under the lock; in principle a completion interrupt (or a delay) could race with the initialization. - KASSERT that the xfer is in progress when we're about to complete it. --- sys/arch/mips/adm5120/dev/ahci.c | 6 +++++- sys/dev/ic/sl811hs.c | 4 ++++ sys/dev/usb/ehci.c | 5 +++-- sys/dev/usb/motg.c | 33 ++++++++++++++++++++++++++++++-- sys/dev/usb/ohci.c | 6 ++++-- sys/dev/usb/uhci.c | 5 +++-- sys/dev/usb/vhci.c | 4 ++++ sys/dev/usb/xhci.c | 2 ++ sys/external/bsd/dwc2/dwc2.c | 6 ++++-- 9 files changed, 60 insertions(+), 11 deletions(-) diff --git a/sys/arch/mips/adm5120/dev/ahci.c b/sys/arch/mips/adm5120/dev/ahci.c index 1c4905f2144a..26aa1027e8d4 100644 --- a/sys/arch/mips/adm5120/dev/ahci.c +++ b/sys/arch/mips/adm5120/dev/ahci.c @@ -438,6 +438,7 @@ ahci_poll_hub(void *arg) xfer = sc->sc_intr_xfer; if (xfer == NULL) goto out; + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); /* * If the intr xfer for which we were scheduled is done, and @@ -754,11 +755,14 @@ ahci_root_intr_start(struct usbd_xfer *xfer) DPRINTF(D_TRACE, ("SLRIstart ")); + mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intr_xfer == NULL); - sc->sc_interval = MS_TO_TICKS(xfer->ux_pipe->up_endpoint->ue_edesc->bInterval); callout_schedule(&sc->sc_poll_handle, sc->sc_interval); sc->sc_intr_xfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; + mutex_exit(&sc->sc_lock); + return USBD_IN_PROGRESS; } diff --git a/sys/dev/ic/sl811hs.c b/sys/dev/ic/sl811hs.c index f20337c33c81..ad2c0006dc7d 100644 --- a/sys/dev/ic/sl811hs.c +++ b/sys/dev/ic/sl811hs.c @@ -1030,7 +1030,9 @@ slhci_root_start(struct usbd_xfer *xfer) KASSERT(spipe->ptype == PT_ROOT_INTR); mutex_enter(&sc->sc_intr_lock); + KASSERT(t->rootintr == NULL); t->rootintr = xfer; + xfer->ux_status = USBD_IN_PROGRESS; mutex_exit(&sc->sc_intr_lock); return USBD_IN_PROGRESS; @@ -2389,6 +2391,8 @@ slhci_callback(struct slhci_softc *sc) if (t->rootintr != NULL) { u_char *p; + KASSERT(t->rootintr->ux_status == + USBD_IN_PROGRESS); p = t->rootintr->ux_buf; p[0] = 2; t->rootintr->ux_actlen = 1; diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c index e657b0e40d68..ceb4612293d9 100644 --- a/sys/dev/usb/ehci.c +++ b/sys/dev/usb/ehci.c @@ -785,6 +785,7 @@ ehci_pcd(void *addr) /* Just ignore the change. */ goto done; } + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); p = xfer->ux_buf; m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1); @@ -2724,11 +2725,11 @@ ehci_root_intr_start(struct usbd_xfer *xfer) mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intrxfer == NULL); sc->sc_intrxfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); - xfer->ux_status = USBD_IN_PROGRESS; - return xfer->ux_status; + return USBD_IN_PROGRESS; } /* Abort a root interrupt request. */ diff --git a/sys/dev/usb/motg.c b/sys/dev/usb/motg.c index 9c2235c5f952..57c61639e9ff 100644 --- a/sys/dev/usb/motg.c +++ b/sys/dev/usb/motg.c @@ -991,8 +991,16 @@ motg_root_intr_abort(struct usbd_xfer *xfer) KASSERT(mutex_owned(&sc->sc_lock)); KASSERT(xfer->ux_pipe->up_intrxfer == xfer); - sc->sc_intr_xfer = NULL; + /* If xfer has already completed, nothing to do here. */ + if (sc->sc_intr_xfer == NULL) + return; + /* + * Otherwise, sc->sc_intr_xfer had better be this transfer. + * Cancel it. + */ + KASSERT(sc->sc_intr_xfer == xfer); + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); xfer->ux_status = USBD_CANCELLED; usb_transfer_complete(xfer); } @@ -1032,7 +1040,14 @@ motg_root_intr_start(struct usbd_xfer *xfer) if (sc->sc_dying) return USBD_IOERROR; + if (!polling) + mutex_enter(&sc->sc_lock); + KASSERT(sc->sc_intr_xfer == NULL); sc->sc_intr_xfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; + if (!polling) + mutex_exit(&sc->sc_lock); + return USBD_IN_PROGRESS; } @@ -1045,12 +1060,25 @@ motg_root_intr_close(struct usbd_pipe *pipe) KASSERT(mutex_owned(&sc->sc_lock)); - sc->sc_intr_xfer = NULL; + /* + * Caller must guarantee the xfer has completed first, by + * closing the pipe only after normal completion or an abort. + */ + KASSERT(sc->sc_intr_xfer == NULL); } void motg_root_intr_done(struct usbd_xfer *xfer) { + struct motg_softc *sc = MOTG_PIPE2SC(pipe); + MOTGHIST_FUNC(); MOTGHIST_CALLED(); + + KASSERT(mutex_owned(&sc->sc_lock)); + + /* Claim the xfer so it doesn't get completed again. */ + KASSERT(sc->sc_intr_xfer == xfer); + KASSERT(xfer->ux_status != USBD_IN_PROGRESS); + sc->sc_intr_xfer = NULL; } void @@ -1101,6 +1129,7 @@ motg_hub_change(struct motg_softc *sc) if (xfer == NULL) return; /* the interrupt pipe is not open */ + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); pipe = xfer->ux_pipe; if (pipe->up_dev == NULL || pipe->up_dev->ud_bus == NULL) diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c index bd437d36135c..6f10fc537d1f 100644 --- a/sys/dev/usb/ohci.c +++ b/sys/dev/usb/ohci.c @@ -1698,6 +1698,8 @@ ohci_rhsc(ohci_softc_t *sc, struct usbd_xfer *xfer) /* Just ignore the change. */ return; } + KASSERT(xfer == sc->sc_intrxfer); + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); p = xfer->ux_buf; m = uimin(sc->sc_noport, xfer->ux_length * 8 - 1); @@ -2516,11 +2518,11 @@ ohci_root_intr_start(struct usbd_xfer *xfer) mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intrxfer == NULL); sc->sc_intrxfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); - xfer->ux_status = USBD_IN_PROGRESS; - return xfer->ux_status; + return USBD_IN_PROGRESS; } /* Abort a root interrupt request. */ diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c index 4d255adfb453..d680cf97cb15 100644 --- a/sys/dev/usb/uhci.c +++ b/sys/dev/usb/uhci.c @@ -1013,6 +1013,7 @@ uhci_poll_hub(void *addr) xfer = sc->sc_intr_xfer; if (xfer == NULL) goto out; + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); /* * If the intr xfer for which we were scheduled is done, and @@ -3905,12 +3906,12 @@ uhci_root_intr_start(struct usbd_xfer *xfer) sc->sc_ival = mstohz(ival); callout_schedule(&sc->sc_poll_handle, sc->sc_ival); sc->sc_intr_xfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); - xfer->ux_status = USBD_IN_PROGRESS; - return xfer->ux_status; + return USBD_IN_PROGRESS; } /* Close the root interrupt pipe. */ diff --git a/sys/dev/usb/vhci.c b/sys/dev/usb/vhci.c index f1c43919d0c3..c0a01b7e344b 100644 --- a/sys/dev/usb/vhci.c +++ b/sys/dev/usb/vhci.c @@ -629,6 +629,7 @@ vhci_root_intr_start(struct usbd_xfer *xfer) mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intrxfer == NULL); sc->sc_intrxfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); @@ -654,6 +655,7 @@ vhci_root_intr_abort(struct usbd_xfer *xfer) * Cancel it. */ KASSERT(sc->sc_intrxfer == xfer); + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); xfer->ux_status = USBD_CANCELLED; usb_transfer_complete(xfer); } @@ -748,6 +750,7 @@ vhci_usb_attach(vhci_fd_t *vfd, struct vhci_ioc_usb_attach *args) ret = ENOBUFS; goto done; } + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); p = xfer->ux_buf; memset(p, 0, xfer->ux_length); @@ -823,6 +826,7 @@ vhci_usb_detach(vhci_fd_t *vfd, struct vhci_ioc_usb_detach *args) mutex_exit(&sc->sc_lock); return ENOBUFS; } + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); mutex_enter(&port->lock); diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c index ebd97494630e..7b8cf33380bd 100644 --- a/sys/dev/usb/xhci.c +++ b/sys/dev/usb/xhci.c @@ -1879,6 +1879,7 @@ xhci_rhpsc(struct xhci_softc * const sc, u_int ctlrport) if (xfer == NULL) return; + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); uint8_t *p = xfer->ux_buf; memset(p, 0, xfer->ux_length); @@ -3717,6 +3718,7 @@ xhci_root_intr_start(struct usbd_xfer *xfer) mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intrxfer[bn] == NULL); sc->sc_intrxfer[bn] = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c index d305c327a34f..0487c55e00e3 100644 --- a/sys/external/bsd/dwc2/dwc2.c +++ b/sys/external/bsd/dwc2/dwc2.c @@ -301,6 +301,8 @@ dwc2_rhc(void *addr) return; } + KASSERT(xfer->ux_status == USBD_IN_PROGRESS); + /* set port bit */ p = KERNADDR(&xfer->ux_dmabuf, 0); @@ -646,11 +648,11 @@ dwc2_root_intr_start(struct usbd_xfer *xfer) mutex_enter(&sc->sc_lock); KASSERT(sc->sc_intrxfer == NULL); sc->sc_intrxfer = xfer; + xfer->ux_status = USBD_IN_PROGRESS; if (!polling) mutex_exit(&sc->sc_lock); - xfer->ux_status = USBD_IN_PROGRESS; - return xfer->ux_status; + return USBD_IN_PROGRESS; } /* Abort a root interrupt request. */