Merge pull request #3817 from hathach/claude/usbd-setup-queue-leak

usbd: fix queued-setup counter leak when the event queue is full

2 files changed, 99 insertions, 2 deletions

diff --git a/src/device/usbd.c b/src/device/usbd.c
index 5471e132d..f5c3046d6 100644
--- a/src/device/usbd.c
+++ b/src/device/usbd.c
@@ -642,6 +642,8 @@ static void configuration_reset(uint8_t rhport) {
 
 static void usbd_reset(uint8_t rhport) {
   configuration_reset(rhport);
+  // discard any pre-reset SETUP still counted: a stale count skips post-reset SETUPs
+  _usbd_queued_setup = 0;
 }
 
 bool tud_task_event_ready(void) {
@@ -1473,8 +1475,18 @@ TU_ATTR_FAST_FUNC void dcd_event_handler(dcd_event_t const* event, bool in_isr)
       break;
   }
 
-  if (send) {
-    queue_event(event, in_isr);
+  if (send && !queue_event(event, in_isr)) {
+    // event dropped by a full queue: undo state that would otherwise wedge permanently
+    if (event->event_id == DCD_EVENT_SETUP_RECEIVED) {
+      // undo the increment, else every later SETUP is skipped as "other SETUP in queue"
+      // and EP0 is deaf until re-init
+      _usbd_queued_setup--;
+    } else if (event->event_id == DCD_EVENT_XFER_COMPLETE) {
+      // clear busy + claimed, else the endpoint can never be claimed or re-armed again
+      uint8_t const epnum = tu_edpt_number(event->xfer_complete.ep_addr);
+      uint8_t const ep_dir = tu_edpt_dir(event->xfer_complete.ep_addr);
+      _usbd_dev.ep_status[epnum][ep_dir] &= (uint8_t) ~(TU_EDPT_STATE_BUSY | TU_EDPT_STATE_CLAIMED);
+    }
   }
 }
 
diff --git a/test/unit-test/test/device/usbd/test_usbd.c b/test/unit-test/test/device/usbd/test_usbd.c
index 7f3c3f5b2..849097326 100644
--- a/test/unit-test/test/device/usbd/test_usbd.c
+++ b/test/unit-test/test/device/usbd/test_usbd.c
@@ -29,6 +29,7 @@
 #include "tusb_fifo.h"
 #include "tusb.h"
 #include "usbd.h"
+#include "device/usbd_pvt.h"
 TEST_SOURCE_FILE("usbd.c")
 
 // Mock File
@@ -271,6 +272,90 @@ void test_usbd_control_in_zlp(void)
 }
 
 //--------------------------------------------------------------------+
+// SETUP dropped by full event queue
+//--------------------------------------------------------------------+
+
+// When the event queue is full, queue_event() drops the SETUP event. The queued-setup
+// counter must not keep the dropped SETUP's increment: a leaked count makes the handler
+// skip every later SETUP ("other SETUP in queue") forever, leaving EP0 permanently deaf.
+void test_usbd_setup_dropped_by_full_queue_recovers(void)
+{
+  // fillers drain through usbd_reset -> class reset
+  mscd_reset_Ignore();
+
+  // fill the queue to the brim, then post one more SETUP: queue_event() drops it
+  for (unsigned i = 0; i < CFG_TUD_TASK_QUEUE_SZ; i++) {
+    dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, false);
+  }
+  dcd_event_setup_received(rhport, (uint8_t*) &req_get_desc_device, false);
+
+  // drain all fillers (each tud_task pass handles at most CFG_TUD_TASK_EVENTS_PER_RUN
+  // events); the dropped SETUP never arrives
+  for (unsigned i = 0; i < (CFG_TUD_TASK_QUEUE_SZ / CFG_TUD_TASK_EVENTS_PER_RUN) + 1; i++) {
+    tud_task();
+  }
+
+  // the next SETUP must still be answered
+  desc_device = (uint8_t const*) &data_desc_device;
+  dcd_event_setup_received(rhport, (uint8_t*) &req_get_desc_device, false);
+
+  dcd_edpt_xfer_ExpectWithArrayAndReturn(rhport, 0x80, (uint8_t*) &data_desc_device, sizeof(tusb_desc_device_t), sizeof(tusb_desc_device_t), false, true);
+  dcd_event_xfer_complete(rhport, EDPT_CTRL_IN, sizeof(tusb_desc_device_t), 0, false);
+
+  dcd_edpt_xfer_ExpectAndReturn(rhport, EDPT_CTRL_OUT, NULL, 0, false, true);
+  dcd_event_xfer_complete(rhport, EDPT_CTRL_OUT, 0, 0, false);
+  dcd_edpt0_status_complete_ExpectWithArray(rhport, &req_get_desc_device, 1);
+
+  tud_task();
+}
+
+//--------------------------------------------------------------------+
+// Transfer completion dropped by full event queue
+//--------------------------------------------------------------------+
+
+// When the event queue is full, queue_event() drops the XFER_COMPLETE event. The endpoint's
+// busy/claimed state must not survive the dropped completion: a leaked BUSY makes every later
+// usbd_edpt_claim()/usbd_edpt_xfer() on that endpoint fail, so the class never re-arms it.
+void test_usbd_xfer_complete_dropped_by_full_queue_recovers(void)
+{
+  // fillers drain through usbd_reset -> class reset
+  mscd_reset_Ignore();
+
+  // open + claim + arm a bulk OUT endpoint the way a class driver would
+  tusb_desc_endpoint_t desc_ep = {
+    .bLength          = sizeof(tusb_desc_endpoint_t),
+    .bDescriptorType  = TUSB_DESC_ENDPOINT,
+    .bEndpointAddress = 0x01,
+    .bmAttributes     = { .xfer = TUSB_XFER_BULK },
+    .wMaxPacketSize   = 64,
+    .bInterval        = 0
+  };
+  static uint8_t xfer_buf[64];
+
+  dcd_edpt_open_ExpectAndReturn(rhport, &desc_ep, true);
+  TEST_ASSERT_TRUE(usbd_edpt_open(rhport, &desc_ep));
+  TEST_ASSERT_TRUE(usbd_edpt_claim(rhport, 0x01));
+  dcd_edpt_xfer_ExpectAndReturn(rhport, 0x01, xfer_buf, 64, false, true);
+  TEST_ASSERT_TRUE(usbd_edpt_xfer(rhport, 0x01, xfer_buf, 64, false));
+
+  // fill the queue to the brim, then complete the transfer: queue_event() drops it
+  for (unsigned i = 0; i < CFG_TUD_TASK_QUEUE_SZ; i++) {
+    dcd_event_bus_signal(rhport, DCD_EVENT_UNPLUGGED, false);
+  }
+  dcd_event_xfer_complete(rhport, 0x01, 64, XFER_RESULT_SUCCESS, false);
+
+  // the endpoint must be re-armable: the dropped completion must not leak busy/claimed
+  TEST_ASSERT_TRUE(usbd_edpt_claim(rhport, 0x01));
+  dcd_edpt_xfer_ExpectAndReturn(rhport, 0x01, xfer_buf, 64, false, true);
+  TEST_ASSERT_TRUE(usbd_edpt_xfer(rhport, 0x01, xfer_buf, 64, false));
+
+  // drain the fillers so later tests start from an empty queue
+  for (unsigned i = 0; i < (CFG_TUD_TASK_QUEUE_SZ / CFG_TUD_TASK_EVENTS_PER_RUN) + 1; i++) {
+    tud_task();
+  }
+}
+
+//--------------------------------------------------------------------+
 // Control OUT data stage host overrun
 //--------------------------------------------------------------------+