summaryrefslogtreecommitdiff
path: root/sys/dev
diff options
context:
space:
mode:
authorriastradh <riastradh@NetBSD.org>2022-05-14 19:44:26 +0000
committerriastradh <riastradh@NetBSD.org>2022-05-14 19:44:26 +0000
commitb27b84a0cbe1e796255aa96fd0b30df46f68c03c (patch)
treec48bb241de4da2605623cb838db9029919e29960 /sys/dev
parenta9f0872b9737416a3c67fa6ab955dfcaf6876239 (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.c24
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);
}
}