diff options
| author | riastradh <riastradh@NetBSD.org> | 2022-05-14 19:44:26 +0000 |
|---|---|---|
| committer | riastradh <riastradh@NetBSD.org> | 2022-05-14 19:44:26 +0000 |
| commit | b27b84a0cbe1e796255aa96fd0b30df46f68c03c (patch) | |
| tree | c48bb241de4da2605623cb838db9029919e29960 /sys/dev | |
| parent | a9f0872b9737416a3c67fa6ab955dfcaf6876239 (diff) | |
xhci(4): Fix edge case in simultaneous xfer abort and failure.
On successful usbd_xfer_trycomplete, caller must set ux_status and
call usb_transfer_complete before releasing the pipe (bus) lock.
Failing to call usb_transfer_complete is a mistake. Presumably this
was intended to claim the xfer to complete it only on the last
packet.
I previously introduced the violation of this rule when the code
looked like
xfer->ux_status = err;
if (trb stuff)
usb_transfer_complete(xfer);
I mostly mechanically changed all the assignments of xfer->ux_status
to do usbd_xfer_trycomplete first and then usb_transfer_complete.
In the original, the extra assignment of xfer->ux_status in the event
we _don't_ immediately call usb_transfer_complete was likely
redundant and (except insofar as the abort protocol was broken)
harmless. But now it is a problem because of the contract between
usbd_xfer_trycomplete and usb_transfer_complete under the pipe (bus)
lock. In retrospect, the original probably should have been
if (trb stuff) {
xfer->ux_status = err;
usb_transfer_complete(xfer);
}
and my mechanical transformation should have worked, but also in
retrospect I should have put more thought into the change and done it
a little less mechanically.
Diffstat (limited to 'sys/dev')
| -rw-r--r-- | sys/dev/usb/xhci.c | 24 |
1 files changed, 12 insertions, 12 deletions
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c index 0e51a78891a..3de0bc815cc 100644 --- a/sys/dev/usb/xhci.c +++ b/sys/dev/usb/xhci.c @@ -1,4 +1,4 @@ -/* $NetBSD: xhci.c,v 1.164 2022/04/06 22:01:45 mlelstv Exp $ */ +/* $NetBSD: xhci.c,v 1.165 2022/05/14 19:44:26 riastradh Exp $ */ /* * Copyright (c) 2013 Jonathan A. Kollasch @@ -34,7 +34,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.164 2022/04/06 22:01:45 mlelstv Exp $"); +__KERNEL_RCSID(0, "$NetBSD: xhci.c,v 1.165 2022/05/14 19:44:26 riastradh Exp $"); #ifdef _KERNEL_OPT #include "opt_usb.h" @@ -2473,18 +2473,18 @@ xhci_event_transfer(struct xhci_softc * const sc, break; } - /* - * Try to claim this xfer for completion. If it has already - * completed or aborted, drop it on the floor. - */ - if (!usbd_xfer_trycomplete(xfer)) - return; - - /* Set the status. */ - xfer->ux_status = err; - if ((trb_3 & XHCI_TRB_3_ED_BIT) == 0 || (trb_0 & 0x3) == 0x0) { + /* + * Try to claim this xfer for completion. If it has + * already completed or aborted, drop it on the floor. + */ + if (!usbd_xfer_trycomplete(xfer)) + return; + + /* Set the status. */ + xfer->ux_status = err; + usb_transfer_complete(xfer); } } |
