Skip to content

Commit e9ba17b

Browse files
fix(dcd): Fixed race condition on device disconnect
TinyUSB does not provide any locking means to protect the DCD variables. This can lead to race conditions when the user is trying to submit a transfer while the device is being disconnected. This can cause the device to be in an inconsistent state, leading to a crash or undefined behavior. This commit adds a spin-lock to protect the DCD variables during device disconnect. Closes espressif/esp-idf#9691 Also reported in espressif/esp-usb#131
1 parent c83fb98 commit e9ba17b

File tree

1 file changed

+28
-0
lines changed

1 file changed

+28
-0
lines changed

src/portable/synopsys/dwc2/dcd_dwc2.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040

4141
#include "device/dcd.h"
4242
#include "dwc2_common.h"
43+
#include "freertos/FreeRTOS.h"
44+
45+
static portMUX_TYPE dcd_lock = portMUX_INITIALIZER_UNLOCKED;
46+
#define DCD_ENTER_CRITICAL() portENTER_CRITICAL(&dcd_lock)
47+
#define DCD_EXIT_CRITICAL() portEXIT_CRITICAL(&dcd_lock)
4348

4449
#if TU_CHECK_MCU(OPT_MCU_GD32VF103)
4550
#define DWC2_EP_COUNT(_dwc2) DWC2_EP_MAX
@@ -58,6 +63,9 @@ typedef struct {
5863
uint8_t interval;
5964
} xfer_ctl_t;
6065

66+
/*
67+
This variable is modified from ISR context, so it must be protected by critical section
68+
*/
6169
static xfer_ctl_t xfer_status[DWC2_EP_MAX][2];
6270
#define XFER_CTL_BASE(_ep, _dir) (&xfer_status[_ep][_dir])
6371

@@ -321,6 +329,9 @@ static void edpt_disable(uint8_t rhport, uint8_t ep_addr, bool stall) {
321329
}
322330
}
323331

332+
// Since this function returns void, it is not possible to return a boolean success message
333+
// We must make sure that this function is not called when the EP is disabled
334+
// Must be called from critical section
324335
static void edpt_schedule_packets(uint8_t rhport, const uint8_t epnum, const uint8_t dir) {
325336
dwc2_regs_t* dwc2 = DWC2_REG(rhport);
326337
xfer_ctl_t* const xfer = XFER_CTL_BASE(epnum, dir);
@@ -540,6 +551,7 @@ void dcd_edpt_close_all(uint8_t rhport) {
540551
dwc2_regs_t* dwc2 = DWC2_REG(rhport);
541552
uint8_t const ep_count = _dwc2_controller[rhport].ep_count;
542553

554+
DCD_ENTER_CRITICAL();
543555
_dcd_data.allocated_epin_count = 0;
544556

545557
// Disable non-control interrupt
@@ -559,6 +571,7 @@ void dcd_edpt_close_all(uint8_t rhport) {
559571
dfifo_flush_rx(dwc2);
560572

561573
dfifo_device_init(rhport); // re-init dfifo
574+
DCD_EXIT_CRITICAL();
562575
}
563576

564577
bool dcd_edpt_iso_alloc(uint8_t rhport, uint8_t ep_addr, uint16_t largest_packet_size) {
@@ -577,7 +590,12 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t to
577590
uint8_t const epnum = tu_edpt_number(ep_addr);
578591
uint8_t const dir = tu_edpt_dir(ep_addr);
579592

593+
DCD_ENTER_CRITICAL();
580594
xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, dir);
595+
if (xfer->max_size == 0) {
596+
DCD_EXIT_CRITICAL();
597+
return false; // Endpoint is closed
598+
}
581599
xfer->buffer = buffer;
582600
xfer->ff = NULL;
583601
xfer->total_len = total_bytes;
@@ -589,6 +607,7 @@ bool dcd_edpt_xfer(uint8_t rhport, uint8_t ep_addr, uint8_t* buffer, uint16_t to
589607

590608
// Schedule packets to be sent within interrupt
591609
edpt_schedule_packets(rhport, epnum, dir);
610+
DCD_EXIT_CRITICAL();
592611

593612
return true;
594613
}
@@ -604,14 +623,20 @@ bool dcd_edpt_xfer_fifo(uint8_t rhport, uint8_t ep_addr, tu_fifo_t* ff, uint16_t
604623
uint8_t const epnum = tu_edpt_number(ep_addr);
605624
uint8_t const dir = tu_edpt_dir(ep_addr);
606625

626+
DCD_ENTER_CRITICAL();
607627
xfer_ctl_t* xfer = XFER_CTL_BASE(epnum, dir);
628+
if (xfer->max_size == 0) {
629+
DCD_EXIT_CRITICAL();
630+
return false; // Endpoint is closed
631+
}
608632
xfer->buffer = NULL;
609633
xfer->ff = ff;
610634
xfer->total_len = total_bytes;
611635

612636
// Schedule packets to be sent within interrupt
613637
// TODO xfer fifo may only available for slave mode
614638
edpt_schedule_packets(rhport, epnum, dir);
639+
DCD_EXIT_CRITICAL();
615640

616641
return true;
617642
}
@@ -640,6 +665,7 @@ void dcd_edpt_clear_stall(uint8_t rhport, uint8_t ep_addr) {
640665
//--------------------------------------------------------------------
641666

642667
// 7.4.1 Initialization on USB Reset
668+
// Must be called from critical section
643669
static void handle_bus_reset(uint8_t rhport) {
644670
dwc2_regs_t *dwc2 = DWC2_REG(rhport);
645671
const uint8_t ep_count = DWC2_EP_COUNT(dwc2);
@@ -990,8 +1016,10 @@ void dcd_int_handler(uint8_t rhport) {
9901016

9911017
if (gintsts & GINTSTS_USBRST) {
9921018
// USBRST is start of reset.
1019+
DCD_ENTER_CRITICAL();
9931020
dwc2->gintsts = GINTSTS_USBRST;
9941021
handle_bus_reset(rhport);
1022+
DCD_EXIT_CRITICAL();
9951023
}
9961024

9971025
if (gintsts & GINTSTS_ENUMDNE) {

0 commit comments

Comments
 (0)