tests: usb: device_next: add test for incomplete control transfers with missing data and status stage #91203
Conversation
jfischer-no
added
area: USB
jfischer-no
force-pushed
the
pr-device_next-test-control-request-handling
branch
from
8680cc2 to
c1015f4
Compare
There was a problem hiding this comment.
Confirmed to work on DWC2 in Completer mode (though it still fails in buffer DMA mode) when used together with #91281
| @@ -73,7 +73,8 @@ int usbh_req_setup(struct usb_device *const udev, | |||
|
|
|||
| memcpy(xfer->setup_pkt, &req, sizeof(req)); | |||
|
|
|||
| __ASSERT((buf != NULL && wLength) || (buf == NULL && !wLength), | |||
| __ASSERT(IS_ENABLED(CONFIG_ZTEST) || | |||
There was a problem hiding this comment.
Omitting asserts during tests suggests there is something seriously wrong with the implementation. Alternatively it suggests that the assert is incorrectly used.
| * never reach the stack. | ||
| */ | ||
| LOG_INF("Drop setup packet (%p)", (void *)data->setup); | ||
| net_buf_unref(data->setup); |
There was a problem hiding this comment.
With DWC2 in Buffer DMA mode, it is possible for data->setup to be equal to buf. If this is the case, then the unref here does free buf and there is a NULL pointer dereference a while later.
There was a problem hiding this comment.
I can no longer trigger this with this PR rebased on top of main.
| if (data->stage != CTRL_PIPE_STAGE_SETUP) { | ||
| LOG_INF("Sequence %u not completed", data->stage); | ||
|
|
||
| if (data->stage == CTRL_PIPE_STAGE_DATA_OUT) { |
There was a problem hiding this comment.
With DWC2 in Buffer DMA mode, I can get the setup leak to happen with following data->stage values here:
- CTRL_PIPE_STAGE_DATA_OUT
- CTRL_PIPE_STAGE_STATUS_IN
- CTRL_PIPE_STAGE_ERROR
|
@jfischer-no please rebase |
The previous setup packet reference will simply be overwritten when the host omits data (OUT) stage. If a new setup packet arrives before the previous data stage is complete, free the last setup packet buffer. Signed-off-by: Johann Fischer <[email protected]>
Drop the queued control transfer when a new setup packet arrives. Signed-off-by: Johann Fischer <[email protected]>
For testing purposes, allow the status stage to be omitted. Signed-off-by: Johann Fischer <[email protected]>
Ignore assers in the control request handling when Kconfig option ZTEST is enabled. Signed-off-by: Johann Fischer <[email protected]>
Add a test for incomplete control transfers with missing data and status stages. Signed-off-by: Johann Fischer <[email protected]>
Add test for the string descriptors. Signed-off-by: Johann Fischer <[email protected]>
Do not use qemu_cortex_m3 for the "build all" case, as it overflows and provides no benefit over native_sim. Add native_sim/native/64 platform to the tests. Signed-off-by: Johann Fischer <[email protected]>
jfischer-no
force-pushed
the
pr-device_next-test-control-request-handling
branch
from
c1015f4 to
4401a0f
Compare
|
Add test for incomplete control transfers with missing data and status stage.
Fix UDC buffer leak when the host omits control data stage.