From 40f01b8f3225ee6c4d88cb0cbd9cc44b5a78264c Mon Sep 17 00:00:00 2001 From: hathach Date: Mon, 15 Jun 2026 23:19:38 +0700 Subject: [PATCH 1/2] device: clamp EP0 OUT data copy to the control transfer buffer usbd_control_xfer_cb() copied xferred_bytes from the EP0 bounce buffer into the requester's buffer with no bound. A non-compliant host that sends an OUT data packet larger than the control transfer's data_len (= min(len, wLength), the buffer capacity) would overflow that buffer and over-count total_xferred. Clamp xferred_bytes to the remaining buffer space before the memcpy and accounting. This is in the shared control handler, so it hardens every DCD. Found while code-reviewing the MUSB EP0 path in #3699. Co-Authored-By: Claude Fable 5 --- src/device/usbd.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/device/usbd.c b/src/device/usbd.c index 55ad330c1a..5e3e5398b2 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -914,6 +914,12 @@ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t // Data stage progress if (ctrl_xfer->request.bmRequestType_bit.direction == TUSB_DIR_OUT) { TU_VERIFY(ctrl_xfer->buffer); + // A non-compliant host can send more than requested; clamp to the remaining buffer so we + // never copy past the caller's buffer (data_len = min(len, wLength) is its capacity). + uint16_t const remain = ctrl_xfer->data_len - ctrl_xfer->total_xferred; + if (xferred_bytes > remain) { + xferred_bytes = remain; + } if (ctrl_xfer->buffer != _ctrl_epbuf.buf) { memcpy(ctrl_xfer->buffer, _ctrl_epbuf.buf, xferred_bytes); } From dcfdd96ad341d2f7c45c244e082472558d63cd8a Mon Sep 17 00:00:00 2001 From: hathach Date: Tue, 16 Jun 2026 15:17:15 +0700 Subject: [PATCH 2/2] device: simplify EP0 OUT clamp with tu_min32, add overrun test Collapse the hand-rolled remain/if clamp in usbd_control_xfer_cb() into tu_min32(), matching the tu_min16 idiom data_stage_xact() already uses for the same "clamp to remaining EP0 capacity" computation, and shorten the comment to the non-obvious why. No behavior change. Add test_usbd_control_out_overrun_clamp covering the host-overrun case: a vendor OUT request offering an 8-byte buffer while the DCD reports a full max-size packet must complete via the IN status stage instead of re-arming an OUT data packet. Also fix the stale TEST_SOURCE_FILE("usbd_control.c") reference (merged into usbd.c) to keep the control path compiled from source. ceedling test:all (61 passed); cdc_msc builds for stm32f407disco. Co-Authored-By: Claude Fable 5 --- src/device/usbd.c | 8 +--- test/unit-test/test/device/usbd/test_usbd.c | 52 ++++++++++++++++++++- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/src/device/usbd.c b/src/device/usbd.c index 5e3e5398b2..f87b63111b 100644 --- a/src/device/usbd.c +++ b/src/device/usbd.c @@ -914,12 +914,8 @@ static bool usbd_control_xfer_cb(uint8_t rhport, uint8_t ep_addr, xfer_result_t // Data stage progress if (ctrl_xfer->request.bmRequestType_bit.direction == TUSB_DIR_OUT) { TU_VERIFY(ctrl_xfer->buffer); - // A non-compliant host can send more than requested; clamp to the remaining buffer so we - // never copy past the caller's buffer (data_len = min(len, wLength) is its capacity). - uint16_t const remain = ctrl_xfer->data_len - ctrl_xfer->total_xferred; - if (xferred_bytes > remain) { - xferred_bytes = remain; - } + // Clamp host overrun to remaining capacity (data_len) so memcpy can't overflow the caller buffer + xferred_bytes = tu_min32(xferred_bytes, ctrl_xfer->data_len - ctrl_xfer->total_xferred); if (ctrl_xfer->buffer != _ctrl_epbuf.buf) { memcpy(ctrl_xfer->buffer, _ctrl_epbuf.buf, xferred_bytes); } diff --git a/test/unit-test/test/device/usbd/test_usbd.c b/test/unit-test/test/device/usbd/test_usbd.c index 3a2cf32178..7f3c3f5b2c 100644 --- a/test/unit-test/test/device/usbd/test_usbd.c +++ b/test/unit-test/test/device/usbd/test_usbd.c @@ -29,7 +29,7 @@ #include "tusb_fifo.h" #include "tusb.h" #include "usbd.h" -TEST_SOURCE_FILE("usbd_control.c") +TEST_SOURCE_FILE("usbd.c") // Mock File #include "mock_dcd.h" @@ -100,6 +100,16 @@ tusb_control_request_t const req_get_desc_configuration = .wLength = 256 }; +// Vendor OUT control request (direction OUT, type Vendor, recipient Device), 8-byte data stage +tusb_control_request_t const req_vendor_out = +{ + .bmRequestType = 0x40, + .bRequest = 0x01, + .wValue = 0x0000, + .wIndex = 0x0000, + .wLength = 8 +}; + uint8_t const* desc_device; uint8_t const* desc_configuration; @@ -120,6 +130,19 @@ uint16_t const* tud_descriptor_string_cb(uint8_t index, uint16_t langid) { return NULL; } +// Backing buffer for the vendor OUT data stage. Sized to EP0 max packet so an (untested) regression +// that drops the clamp can't corrupt memory here; the regression is caught by the expectation below. +static uint8_t vendor_out_buf[CFG_TUD_ENDPOINT0_SIZE]; + +bool tud_vendor_control_xfer_cb(uint8_t rhport_, uint8_t stage, tusb_control_request_t const* request) { + (void) request; + if (stage == CONTROL_STAGE_SETUP) { + // Offer only an 8-byte capacity even though the data stage may receive a larger packet + return tud_control_xfer(rhport_, request, vendor_out_buf, 8); + } + return true; +} + void setUp(void) { dcd_int_disable_Ignore(); dcd_int_enable_Ignore(); @@ -246,3 +269,30 @@ void test_usbd_control_in_zlp(void) tud_task(); } + +//--------------------------------------------------------------------+ +// Control OUT data stage host overrun +//--------------------------------------------------------------------+ + +// A non-compliant host sends an OUT data packet larger than the buffer the class offered: +// wLength = 8, but the DCD reports a full CFG_TUD_ENDPOINT0_SIZE packet. usbd must clamp the +// copy/accounting to the 8-byte capacity so total_xferred reaches wLength, ends the data stage, +// and queues the IN status stage. Without the clamp total_xferred overshoots wLength and usbd +// re-arms an OUT data packet (EDPT_CTRL_OUT) instead, failing the EDPT_CTRL_IN expectation below. +void test_usbd_control_out_overrun_clamp(void) +{ + dcd_event_setup_received(rhport, (uint8_t*) &req_vendor_out, false); + + // Data stage: usbd arms an 8-byte OUT into its internal bounce buffer (buffer ptr is internal) + dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_OUT, NULL, 8, false, true); + dcd_edpt_xfer_IgnoreArg_buffer(); + // Host overrun: DCD reports a full max packet, larger than the 8-byte capacity + dcd_event_xfer_complete(rhport, EDPT_CTRL_OUT, CFG_TUD_ENDPOINT0_SIZE, XFER_RESULT_SUCCESS, false); + + // Clamp -> total_xferred == wLength -> data stage done -> IN status stage queued + dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_IN, NULL, 0, false, true); + dcd_event_xfer_complete(rhport, EDPT_CTRL_IN, 0, 0, false); + dcd_edpt0_status_complete_ExpectWithArray(rhport, &req_vendor_out, 1); + + tud_task(); +}