From 23b82a560e2bc015bbe281bddd9c92309bfe4c7f Mon Sep 17 00:00:00 2001 From: Taylor R Campbell Date: Mon, 21 Dec 2020 13:52:36 +0000 Subject: [PATCH] usb: Maintain struct usbd_pipe::up_intrxfer in usbd_intr_transfer. For pipes at input-direction (device-to-host) interrupt-type endpoints, up_intrxfer is set by usbd_open_pipe_intr, and later used only by usbd_close_pipe to free the transfer later on. But for pipes at _output-direction_ (host-to-device) interrupt-type endpoints, nothing previously set up_intrxfer -- yet several HCI drivers would nevertheless assert up_intrxfer == xfer in their abort routines. This triggered when I tried to abort a write to a u2f device. As far as I know, it only makes sense to have one output interrupt transfer at a time. So this patch changes usbd_intr_transfer to set up_intrxfer (and assert that it's exclusive), and sprinkles a few more assertions into the HCI driver callbacks for interrupt pipes to verify that it really is the exclusive interrupt transfer that they're operating on. --- sys/arch/mips/adm5120/dev/ahci.c | 6 ++++++ sys/dev/usb/ehci.c | 4 ++++ sys/dev/usb/ohci.c | 3 +++ sys/dev/usb/uhci.c | 3 +++ sys/dev/usb/usbdi_util.c | 16 ++++++++++++++++ sys/dev/usb/xhci.c | 3 +++ sys/external/bsd/dwc2/dwc2.c | 5 +++++ 7 files changed, 40 insertions(+) diff --git a/sys/arch/mips/adm5120/dev/ahci.c b/sys/arch/mips/adm5120/dev/ahci.c index 874c21c3861a..f86483f545fc 100644 --- a/sys/arch/mips/adm5120/dev/ahci.c +++ b/sys/arch/mips/adm5120/dev/ahci.c @@ -1023,6 +1023,7 @@ ahci_device_intr_transfer(struct usbd_xfer *xfer) DPRINTF(D_TRACE, ("INTRtrans ")); mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); error = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (error) @@ -1039,6 +1040,8 @@ ahci_device_intr_start(struct usbd_xfer *xfer) DPRINTF(D_TRACE, ("INTRstart ")); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); + sx = kmem_intr_alloc(sizeof(*sx), KM_NOSLEEP); if (sx == NULL) goto reterr; @@ -1103,6 +1106,8 @@ ahci_device_intr_abort(struct usbd_xfer *xfer) DPRINTF(D_TRACE, ("INTRabort ")); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); + sx = xfer->ux_hcpriv; if (sx) { callout_stop(&sx->sx_callout_t); @@ -1124,6 +1129,7 @@ static void ahci_device_intr_done(struct usbd_xfer *xfer) { DPRINTF(D_TRACE, ("INTRdone ")); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); } static usbd_status diff --git a/sys/dev/usb/ehci.c b/sys/dev/usb/ehci.c index af0c79cdf315..f04077462703 100644 --- a/sys/dev/usb/ehci.c +++ b/sys/dev/usb/ehci.c @@ -4037,6 +4037,7 @@ ehci_device_intr_transfer(struct usbd_xfer *xfer) /* Insert last in queue. */ mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) @@ -4085,6 +4086,8 @@ ehci_device_intr_start(struct usbd_xfer *xfer) if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); + ehci_reset_sqtd_chain(sc, xfer, len, isread, &epipe->nexttoggle, &end); end->qtd.qtd_status |= htole32(EHCI_QTD_IOC); @@ -4166,6 +4169,7 @@ ehci_device_intr_done(struct usbd_xfer *xfer) DPRINTF("xfer=%#jx, actlen=%jd", (uintptr_t)xfer, xfer->ux_actlen, 0, 0); KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); if (xfer->ux_length) { int isread, endpt; diff --git a/sys/dev/usb/ohci.c b/sys/dev/usb/ohci.c index d7cad46fb6ef..bc15a83c66bc 100644 --- a/sys/dev/usb/ohci.c +++ b/sys/dev/usb/ohci.c @@ -1643,6 +1643,7 @@ ohci_device_intr_done(struct usbd_xfer *xfer) xfer->ux_actlen, 0, 0); KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, isread ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); @@ -3122,6 +3123,7 @@ ohci_device_intr_transfer(struct usbd_xfer *xfer) /* Insert last in queue. */ mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) @@ -3158,6 +3160,7 @@ ohci_device_intr_start(struct usbd_xfer *xfer) if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); /* * Use the pipe "tail" TD as our first and loan our first TD to the diff --git a/sys/dev/usb/uhci.c b/sys/dev/usb/uhci.c index 86274dd13571..07242549cfa8 100644 --- a/sys/dev/usb/uhci.c +++ b/sys/dev/usb/uhci.c @@ -2690,6 +2690,7 @@ uhci_device_intr_transfer(struct usbd_xfer *xfer) /* Insert last in queue. */ mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) @@ -2738,6 +2739,7 @@ uhci_device_intr_start(struct usbd_xfer *xfer) /* Take lock to protect nexttoggle */ if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); uhci_reset_std_chain(sc, xfer, xfer->ux_length, isread, &upipe->nexttoggle, &dataend); @@ -3229,6 +3231,7 @@ uhci_device_intr_done(struct usbd_xfer *xfer) DPRINTFN(5, "length=%jd", xfer->ux_actlen, 0, 0, 0); KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); npoll = upipe->intr.npoll; for (i = 0; i < npoll; i++) { diff --git a/sys/dev/usb/usbdi_util.c b/sys/dev/usb/usbdi_util.c index 01070ace3fd7..3f6c6e0ef725 100644 --- a/sys/dev/usb/usbdi_util.c +++ b/sys/dev/usb/usbdi_util.c @@ -563,10 +563,26 @@ usbd_intr_transfer(struct usbd_xfer *xfer, struct usbd_pipe *pipe, usbd_setup_xfer(xfer, 0, buf, *size, flags, timeout, NULL); + if (!xfer->ux_bus->ub_usepolling) + mutex_enter(xfer->ux_bus->ub_lock); + KASSERTMSG(pipe->up_intrxfer == NULL, + "pipe=%p intrxfer=%p xfer=%p", pipe, pipe->up_intrxfer, xfer); + pipe->up_intrxfer = xfer; + if (!xfer->ux_bus->ub_usepolling) + mutex_exit(xfer->ux_bus->ub_lock); + err = usbd_sync_transfer_sig(xfer); usbd_get_xfer_status(xfer, NULL, NULL, size, NULL); + if (!xfer->ux_bus->ub_usepolling) + mutex_enter(xfer->ux_bus->ub_lock); + KASSERTMSG(pipe->up_intrxfer == xfer, + "pipe=%p intrxfer=%p xfer=%p", pipe, pipe->up_intrxfer, xfer); + pipe->up_intrxfer = NULL; + if (!xfer->ux_bus->ub_usepolling) + mutex_exit(xfer->ux_bus->ub_lock); + DPRINTFN(1, "transferred %jd", *size, 0, 0, 0); if (err) { usbd_clear_endpoint_stall(pipe); diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c index 5e08b64f38de..c0358909f5f7 100644 --- a/sys/dev/usb/xhci.c +++ b/sys/dev/usb/xhci.c @@ -4058,6 +4058,7 @@ xhci_device_intr_transfer(struct usbd_xfer *xfer) /* Insert last in queue. */ mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) @@ -4113,6 +4114,7 @@ xhci_device_intr_start(struct usbd_xfer *xfer) if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); xfer->ux_status = USBD_IN_PROGRESS; xhci_db_write_4(sc, XHCI_DOORBELL(xs->xs_idx), dci); usbd_xfer_schedule_timeout(xfer); @@ -4137,6 +4139,7 @@ xhci_device_intr_done(struct usbd_xfer *xfer) (uintptr_t)xfer, xs->xs_idx, dci, 0); KASSERT(xhci_polling_p(sc) || mutex_owned(&sc->sc_lock)); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); usb_syncmem(&xfer->ux_dmabuf, 0, xfer->ux_length, isread ? BUS_DMASYNC_POSTREAD : BUS_DMASYNC_POSTWRITE); diff --git a/sys/external/bsd/dwc2/dwc2.c b/sys/external/bsd/dwc2/dwc2.c index 94ee15781118..2d8fbbc17493 100644 --- a/sys/external/bsd/dwc2/dwc2.c +++ b/sys/external/bsd/dwc2/dwc2.c @@ -842,6 +842,7 @@ dwc2_device_intr_transfer(struct usbd_xfer *xfer) /* Insert last in queue. */ mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); err = usb_insert_transfer(xfer); mutex_exit(&sc->sc_lock); if (err) @@ -862,6 +863,7 @@ dwc2_device_intr_start(struct usbd_xfer *xfer) if (!polling) mutex_enter(&sc->sc_lock); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); xfer->ux_status = USBD_IN_PROGRESS; err = dwc2_device_start(xfer); if (!polling) @@ -900,6 +902,9 @@ dwc2_device_intr_done(struct usbd_xfer *xfer) { DPRINTF("\n"); + + KASSERT(sc->sc_bus.ub_usepolling || mutex_owned(&sc->sc_lock)); + KASSERT(xfer->ux_pipe->up_intrxfer == xfer); } /***********************************************************************/