hrev52963 adds 3 changesets to branch 'master'
old head: 3fe76c19f50d60356b9902fa4193580b242e51ed
new head: a40a4c0d39da8d0cfe37263d3b82c4ba84414d37
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=a40a4c0d39da+%5E3fe76c19f50d
----------------------------------------------------------------------------
1533bd88b1ba: XHCI: Style cleanup and dead code removal.
No functional change (besides some tweaked tracing) intended.
6b4d0adcd0b8: XHCI: Clear the device struct after tearing down its state.
This clears out the area_ids, the pointers to them, and a variety
of other state information that will be invalid following the
deletion of the device.
Turns #12929 from a use-after-free into a NULL dereference panic.
a40a4c0d39da: XHCI: Don't try to submit transfers to non-existent endpoints.
Fixes #12929. The underlying issue that caused it still remains, though.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
1 file changed, 39 insertions(+), 32 deletions(-)
src/add-ons/kernel/busses/usb/xhci.cpp | 71 ++++++++++++++++--------------
############################################################################
Commit: 1533bd88b1baf82c0aadd1a47f7b6214d30c14dc
URL: https://git.haiku-os.org/haiku/commit/?id=1533bd88b1ba
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Mon Mar 4 04:45:19 2019 UTC
XHCI: Style cleanup and dead code removal.
No functional change (besides some tweaked tracing) intended.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index 395e6a1a40..c70bb63227 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -715,7 +715,8 @@ XHCI::SubmitNormalRequest(Transfer *transfer)
return status;
int32 trbCount = 0;
- xhci_td *descriptor = CreateDescriptorChain(transfer->DataLength(),
trbCount);
+ xhci_td *descriptor = CreateDescriptorChain(transfer->DataLength(),
+ trbCount);
if (descriptor == NULL)
return B_NO_MEMORY;
@@ -739,7 +740,8 @@ XHCI::SubmitNormalRequest(Transfer *transfer)
}
// link next td, if any
if (td_chain->next_chain != NULL) {
- td_chain->trbs[td_chain->trb_count].qwtrb0 =
td_chain->next_chain->this_phy;
+ td_chain->trbs[td_chain->trb_count].qwtrb0 =
+ td_chain->next_chain->this_phy;
td_chain->trbs[td_chain->trb_count].dwtrb2 =
TRB_2_IRQ(0);
td_chain->trbs[td_chain->trb_count].dwtrb3
=
B_HOST_TO_LENDIAN_INT32(TRB_3_TYPE(TRB_TYPE_LINK)
@@ -822,10 +824,6 @@ XHCI::NotifyPipeChange(Pipe *pipe, usb_change change)
status_t
XHCI::AddTo(Stack *stack)
{
-#ifdef TRACE_USB
- set_dprintf_enabled(true);
-#endif
-
if (!sPCIModule) {
status_t status = get_module(B_PCI_MODULE_NAME,
(module_info **)&sPCIModule);
@@ -923,8 +921,8 @@ XHCI::CreateDescriptorChain(size_t bufferSize, int32
&trbCount)
TRACE("CreateDescriptorChain trbs %d for td %" B_PRId32 "\n",
trbs, i);
for (int j = 0; j < trbs; j++) {
if (fStack->AllocateChunk(&descriptor->buffer_log[j],
- &descriptor->buffer_phy[j],
- min_c(packetSize, bufferSize)) < B_OK) {
+ &descriptor->buffer_phy[j],
+ min_c(packetSize, bufferSize)) < B_OK) {
TRACE_ERROR("unable to allocate space for the
buffer (size %"
B_PRIuSIZE ")\n", bufferSize);
return NULL;
@@ -954,7 +952,7 @@ XHCI::CreateDescriptor(size_t bufferSize)
phys_addr_t physicalAddress;
if (fStack->AllocateChunk((void **)&result, &physicalAddress,
- sizeof(xhci_td)) < B_OK) {
+ sizeof(xhci_td)) < B_OK) {
TRACE_ERROR("failed to allocate a transfer descriptor\n");
return NULL;
}
@@ -972,7 +970,7 @@ XHCI::CreateDescriptor(size_t bufferSize)
}
if (fStack->AllocateChunk(&result->buffer_log[0],
- &result->buffer_phy[0], bufferSize) < B_OK) {
+ &result->buffer_phy[0], bufferSize) < B_OK) {
TRACE_ERROR("unable to allocate space for the buffer (size
%ld)\n",
bufferSize);
fStack->FreeChunk(result, result->this_phy, sizeof(xhci_td));
@@ -980,7 +978,7 @@ XHCI::CreateDescriptor(size_t bufferSize)
}
TRACE("CreateDescriptor allocated buffer_size %ld %p\n",
- result->buffer_size[0], result->buffer_log[0]);
+ result->buffer_size[0], result->buffer_log[0]);
return result;
}
@@ -990,7 +988,6 @@ void
XHCI::FreeDescriptor(xhci_td *descriptor)
{
while (descriptor != NULL) {
-
for (int i = 0; i < descriptor->buffer_count; i++) {
if (descriptor->buffer_size[i] == 0)
continue;
@@ -1450,7 +1447,7 @@ XHCI::_InsertEndpointForPipe(Pipe *pipe)
TRACE("_InsertEndpointForPipe endpoint address %" B_PRId8 "\n",
pipe->EndpointAddress());
if (pipe->ControllerCookie() != NULL
- || pipe->Parent()->Type() != USB_OBJECT_DEVICE) {
+ || pipe->Parent()->Type() != USB_OBJECT_DEVICE) {
// default pipe is already referenced
return B_OK;
}
@@ -1523,16 +1520,6 @@ XHCI::_InsertEndpointForPipe(Pipe *pipe)
return status;
}
-#if 0
- /* These commands error with "Context state" on some devices,
- * and on others break transfers altogether. So just disable
them
- * for now. */
- StopEndpoint(false, endpoint, device->slot);
- SetTRDequeue(device->endpoints[id].trb_addr, 0, endpoint,
- device->slot);
- ResetEndpoint(false, endpoint, device->slot);
-#endif
-
_WriteContext(&device->input_ctx->input.dropFlags, 0);
_WriteContext(&device->input_ctx->input.addFlags,
(1 << endpoint) | (1 << 0));
@@ -2011,6 +1998,7 @@ XHCI::Ring(uint8 slot, uint8 endpoint)
panic("Ring() invalid slot/endpoint combination\n");
if (slot > fSlotCount || endpoint >= XHCI_MAX_ENDPOINTS)
panic("Ring() invalid slot or endpoint\n");
+
WriteDoorReg32(XHCI_DOORBELL(slot), XHCI_DOORBELL_TARGET(endpoint)
| XHCI_DOORBELL_STREAMID(0));
/* Flush PCI posted writes */
@@ -2064,6 +2052,8 @@ XHCI::QueueCommand(xhci_trb* trb)
void
XHCI::HandleCmdComplete(xhci_trb* trb)
{
+ TRACE("HandleCmdComplete trb %p\n", trb);
+
if (fCmdAddr == trb->qwtrb0) {
TRACE("Received command event\n");
fCmdResult[0] = trb->dwtrb2;
############################################################################
Commit: 6b4d0adcd0b801dbdfcf5ae358f02b4629fc70c7
URL: https://git.haiku-os.org/haiku/commit/?id=6b4d0adcd0b8
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Mon Mar 4 19:54:08 2019 UTC
Ticket: https://dev.haiku-os.org/ticket/12929
XHCI: Clear the device struct after tearing down its state.
This clears out the area_ids, the pointers to them, and a variety
of other state information that will be invalid following the
deletion of the device.
Turns #12929 from a use-after-free into a NULL dereference panic.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index c70bb63227..89f6885897 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -1232,8 +1232,9 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress, uint8
hubPort,
"XHCI device context");
if (device->device_ctx_area < B_OK) {
TRACE_ERROR("unable to create a device context area\n");
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
memset(device->device_ctx, 0, sizeof(*device->device_ctx) <<
fContextSizeShift);
@@ -1243,9 +1244,10 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress, uint8
hubPort,
* XHCI_MAX_TRANSFERS, "XHCI endpoint trbs");
if (device->trb_area < B_OK) {
TRACE_ERROR("unable to create a device trbs area\n");
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
@@ -1270,10 +1272,11 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress,
uint8 hubPort,
if (ConfigureEndpoint(slot, 0, 4, device->trb_addr, 0,
maxPacketSize, maxPacketSize & 0x7ff, speed) != B_OK) {
TRACE_ERROR("unable to configure default control endpoint\n");
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
delete_area(device->trb_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
@@ -1288,10 +1291,11 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress,
uint8 hubPort,
// device should get to addressed state (bsr = 0)
if (SetAddress(device->input_ctx_addr, false, slot) != B_OK) {
TRACE_ERROR("unable to set address\n");
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
delete_area(device->trb_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
@@ -1333,10 +1337,11 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress,
uint8 hubPort,
if (actualLength != 8) {
TRACE_ERROR("error while getting the device descriptor: %s\n",
strerror(status));
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
delete_area(device->trb_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
@@ -1377,10 +1382,11 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress,
uint8 hubPort,
if (actualLength != sizeof(usb_hub_descriptor)) {
TRACE_ERROR("error while getting the hub descriptor:
%s\n",
strerror(status));
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
delete_area(device->trb_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
@@ -1409,10 +1415,11 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress,
uint8 hubPort,
} else {
TRACE_ERROR("device object failed to initialize\n");
}
- device->state = XHCI_STATE_DISABLED;
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
delete_area(device->trb_area);
+ memset(device, 0, sizeof(xhci_device));
+ device->state = XHCI_STATE_DISABLED;
return NULL;
}
fPortSlots[hubPort] = slot;
@@ -1437,6 +1444,8 @@ XHCI::FreeDevice(Device *device)
delete_area(fDevices[slot].trb_area);
delete_area(fDevices[slot].input_ctx_area);
delete_area(fDevices[slot].device_ctx_area);
+
+ memset(&fDevices[slot], 0, sizeof(xhci_device));
fDevices[slot].state = XHCI_STATE_DISABLED;
}
############################################################################
Revision: hrev52963
Commit: a40a4c0d39da8d0cfe37263d3b82c4ba84414d37
URL: https://git.haiku-os.org/haiku/commit/?id=a40a4c0d39da
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Mon Mar 4 20:37:38 2019 UTC
Ticket: https://dev.haiku-os.org/ticket/12929
XHCI: Don't try to submit transfers to non-existent endpoints.
Fixes #12929. The underlying issue that caused it still remains, though.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index 89f6885897..a7e8690e9f 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -621,7 +621,7 @@ XHCI::SubmitControlRequest(Transfer *transfer)
xhci_endpoint *endpoint = (xhci_endpoint *)pipe->ControllerCookie();
uint8 id = XHCI_ENDPOINT_ID(pipe);
if (id >= XHCI_MAX_ENDPOINTS) {
- TRACE_ERROR("Invalid Endpoint");
+ TRACE_ERROR("invalid endpoint!\n");
return B_BAD_VALUE;
}
status_t status = transfer->InitKernelAccess();
@@ -704,6 +704,7 @@ status_t
XHCI::SubmitNormalRequest(Transfer *transfer)
{
TRACE("SubmitNormalRequest() length %ld\n", transfer->DataLength());
+
Pipe *pipe = transfer->TransferPipe();
uint8 id = XHCI_ENDPOINT_ID(pipe);
if (id >= XHCI_MAX_ENDPOINTS)
@@ -1455,6 +1456,7 @@ XHCI::_InsertEndpointForPipe(Pipe *pipe)
{
TRACE("_InsertEndpointForPipe endpoint address %" B_PRId8 "\n",
pipe->EndpointAddress());
+
if (pipe->ControllerCookie() != NULL
|| pipe->Parent()->Type() != USB_OBJECT_DEVICE) {
// default pipe is already referenced
@@ -1573,6 +1575,12 @@ status_t
XHCI::_LinkDescriptorForPipe(xhci_td *descriptor, xhci_endpoint *endpoint)
{
TRACE("_LinkDescriptorForPipe\n");
+
+ 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) {
TRACE_ERROR("_LinkDescriptorForPipe max transfers count
exceeded\n");
@@ -2133,7 +2141,7 @@ XHCI::HandleTransferComplete(xhci_trb* trb)
return;
}
}
- TRACE_ERROR("TRB %" B_PRIxADDR " was not found in the endpoint!\n",
source);
+ TRACE_ERROR("TRB 0x%" B_PRIxADDR " was not found in the endpoint!\n",
source);
}
