From 58d1d9f9d6f512ce52828a850ac750aa41ceedd2 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 21 Jul 2026 15:26:35 +0200 Subject: [PATCH] drivers/usbdev/cdcacm: serialize the TX ring drain with the class spinlock cdcacm_sndpacket() runs from task context and from the bulk IN completion callback, which may be interrupt context. cdcuart_dmasend() advances the xmit tail non-atomically, so a completion arriving mid setup re-sends the same region and advances the tail past the head, re-transmitting a ring of stale data. c497c5feb0 dropped the critical section that used to cover this. Restore it with priv->lock held across the setup and EP_SUBMIT; the submit must stay inside to keep request order. cdcuart_dmasend() now runs with the lock held, so its own acquisition is removed. The race needs the writer to keep the ring non-empty across completions, so it only appears at high sustained write rates. On nRF52840, 131072-byte writes were received as ~147600 bytes - one extra CDCACM_TXBUFSIZE of stale data per hit. With this change the host receives exactly what was sent. Assisted-by: Claude Code Signed-off-by: raiden00pl --- drivers/usbdev/cdcacm.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/usbdev/cdcacm.c b/drivers/usbdev/cdcacm.c index 5b003218fb6..87671cf0ae4 100644 --- a/drivers/usbdev/cdcacm.c +++ b/drivers/usbdev/cdcacm.c @@ -424,9 +424,9 @@ static int cdcacm_sndpacket(FAR struct cdcacm_dev_s *priv) FAR struct uart_dev_s *dev = &priv->serdev; FAR struct cdcacm_wrreq_s *wrcontainer; FAR struct usbdev_req_s *req; - irqstate_t flags; int ret; #endif + irqstate_t flags; #ifdef CONFIG_DEBUG_FEATURES if (priv == NULL) @@ -485,10 +485,17 @@ static int cdcacm_sndpacket(FAR struct cdcacm_dev_s *priv) spin_unlock_irqrestore_nopreempt(&priv->lock, flags); #else + /* Serialize against the write completion callback, which may call + * this from interrupt context. + */ + + flags = spin_lock_irqsave(&priv->lock); if (!sq_empty(&priv->txfree)) { uart_xmitchars_dma(&priv->serdev); } + + spin_unlock_irqrestore(&priv->lock, flags); #endif out: @@ -3003,6 +3010,8 @@ static int cdcuart_release(FAR struct uart_dev_s *dev) * Description: * Set up to transfer bytes from the TX circular buffer. * + * Called from cdcacm_sndpacket() with priv->lock held. + * ****************************************************************************/ static void cdcuart_dmasend(FAR struct uart_dev_s *dev) @@ -3012,7 +3021,6 @@ static void cdcuart_dmasend(FAR struct uart_dev_s *dev) FAR struct usbdev_ep_s *ep = priv->epbulkin; FAR struct cdcacm_wrreq_s *wrcontainer; FAR struct usbdev_req_s *req; - irqstate_t flags; size_t nbytes; size_t reqlen; int ret; @@ -3023,11 +3031,9 @@ static void cdcuart_dmasend(FAR struct uart_dev_s *dev) /* Peek at the request in the container at the head of the list */ - flags = spin_lock_irqsave(&priv->lock); wrcontainer = (FAR struct cdcacm_wrreq_s *)sq_remfirst(&priv->txfree); req = wrcontainer->req; priv->nwrq--; - spin_unlock_irqrestore(&priv->lock, flags); /* Fill the request with serial TX data */