From: Kirill A. Korinsky <kirill@korins.ky>
Subject: sys/xhci: check completion before handling stop errors
To: OpenBSD tech <tech@openbsd.org>
Date: Tue, 15 Sep 2026 03:42:41 +0200

tech@,

I had replaced battery on one of my laptops and seems that I broke embeded
webcam partially, which allows to uncover some new edge cases!

So, here the first one which leads to a crash with output:

video0 detached
uvideo0 detached
[garbage]: could not set alternate interface 7!
[garbage]: could not set alternate interface!
usb_transfer_complete: xfer=0xfffff08460797690 not on queue
panic: pool_do_get: xhcixfer free list modified: page 0xfffff08460797000; item addr 0x...

which had happened only once and it is a kind of a blind shot.

Seems that a transfer can complete while xhci_abort_xfer() waits for the
stop endpoint command; xhci_xfer_done() then clears xp->aborted_xfer, and
the caller can free the transfer.

Thoughts?

Index: sys/dev/usb/xhci.c
===================================================================
RCS file: /home/cvs/src/sys/dev/usb/xhci.c,v
diff -u -p -r1.136 xhci.c
--- sys/dev/usb/xhci.c	1 Mar 2025 14:43:03 -0000	1.136
+++ sys/dev/usb/xhci.c	15 Sep 2026 01:36:27 -0000
@@ -2367,15 +2367,7 @@ xhci_abort_xfer(struct usbd_xfer *xfer, 
 	xp->aborted_xfer = xfer;
 
 	/* Stop the endpoint and wait until the hardware says so. */
-	if (xhci_cmd_stop_ep(sc, xp->slot, xp->dci)) {
-		DPRINTF(("%s: error stopping endpoint\n", DEVNAME(sc)));
-		/* Assume the device is gone. */
-		xp->halted = 0;
-		xp->aborted_xfer = NULL;
-		xfer->status = status;
-		usb_transfer_complete(xfer);
-		return;
-	}
+	error = xhci_cmd_stop_ep(sc, xp->slot, xp->dci);
 
 	/*
 	 * The transfer was already completed when we stopped the
@@ -2385,6 +2377,16 @@ xhci_abort_xfer(struct usbd_xfer *xfer, 
 	if (xp->aborted_xfer == NULL) {
 		DPRINTF(("%s: done before stopping the endpoint\n", __func__));
 		xp->halted = 0;
+		return;
+	}
+
+	if (error) {
+		DPRINTF(("%s: error stopping endpoint\n", DEVNAME(sc)));
+		/* Assume the device is gone. */
+		xp->halted = 0;
+		xp->aborted_xfer = NULL;
+		xfer->status = status;
+		usb_transfer_complete(xfer);
 		return;
 	}
 


-- 
wbr, Kirill