From ee0d30cff96a2cc0c6afa5e124c5ba56bd8744c0 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Tue, 5 Mar 2019 01:12:08 -0500 Subject: [PATCH] XHCI: Fix fencepost logic error in _LinkDescriptorForPipe. See inline comment. This is very subtle stuff... I didn't manage to trigger this in my brief testing, and the USB-HID stall during USB Disk access is still not very difficult to reproduce. So, this doesn't seem to have been the issue. --- src/add-ons/kernel/busses/usb/xhci.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp b/src/add-ons/kernel/busses/usb/xhci.cpp index b911470800..60ed8001b3 100644 --- a/src/add-ons/kernel/busses/usb/xhci.cpp +++ b/src/add-ons/kernel/busses/usb/xhci.cpp @@ -1508,13 +1508,20 @@ XHCI::_LinkDescriptorForPipe(xhci_td *descriptor, xhci_endpoint *endpoint) { TRACE("_LinkDescriptorForPipe\n"); + // We must check this before we lock the endpoint, because if it is + // NULL, the mutex is probably uninitialized, too. if (endpoint->device == NULL) { TRACE_ERROR("trying to submit a transfer to a non-existent endpoint!\n"); return B_NO_INIT; } MutexLocker endpointLocker(endpoint->lock); - if (endpoint->used >= XHCI_MAX_TRANSFERS) { + + // We will be modifying 2 TRBs as part of linking a new descriptor: + // the "current" TRB (which will link to the passed descriptor), and + // the "next" (current + 1) TRB (which will be zeroed, as we have + // likely used it before.) Hence the "+ 1" in this check. + if ((endpoint->used + 1) >= XHCI_MAX_TRANSFERS) { TRACE_ERROR("_LinkDescriptorForPipe max transfers count exceeded\n"); return B_BAD_VALUE; }