From 357adccf9f266005c453d5ed778a0217937aac8d Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 17 Feb 2020 14:33:54 -0500 Subject: [PATCH] XHCI: Rearrange RemoveEndpointForPipe to fix crashes and deadlocks. * As we were clearing out the endpoint structures before detaching the xhci_endpoint from the Pipe, it was possible for SubmitRequest to be called while we were being destroyed and then use the half- torn-down endpoint, leading to crashes. Instead, we now call SetControllerCookie first, so that SubmitRequest will thus fail. * Lock the endpoint after calling StopEndpoint, for the same reasons that this is now the order of operations in CancelQueuedTransfers. Probably fixes #15710, and maybe other issues. --- src/add-ons/kernel/busses/usb/xhci.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp b/src/add-ons/kernel/busses/usb/xhci.cpp index aa3c63cb23..d553072f91 100644 --- a/src/add-ons/kernel/busses/usb/xhci.cpp +++ b/src/add-ons/kernel/busses/usb/xhci.cpp @@ -1765,14 +1765,15 @@ XHCI::_RemoveEndpointForPipe(Pipe *pipe) if (endpoint == NULL || endpoint->trbs == NULL) return B_NO_INIT; - xhci_device *device = endpoint->device; + pipe->SetControllerCookie(NULL); if (endpoint->id > 0) { - mutex_lock(&endpoint->lock); - + xhci_device *device = endpoint->device; uint8 epNumber = endpoint->id + 1; StopEndpoint(true, epNumber, device->slot); + mutex_lock(&endpoint->lock); + // See comment in CancelQueuedTransfers. xhci_td* td; while ((td = endpoint->td_head) != NULL) { @@ -1791,7 +1792,6 @@ XHCI::_RemoveEndpointForPipe(Pipe *pipe) else EvaluateContext(device->input_ctx_addr, device->slot); } - pipe->SetControllerCookie(NULL); return B_OK; }