drivers/usbdev/cdcncm: fix TX corruption/wedge under write buffers

drivers/usbdev/cdcncm.c

@@ -918,14 +918,51 @@
   int ndpindex;
   int totallen;
 
-  /* Wait until the USB device request for Ethernet frame transmissions
-   * becomes available.
+  /* Serialise this transmit against cdcncm_send() and against any other
+   * cdcncm_transmit_work() invocation.  cdcncm_send() runs under netdev_lock
+   * (the recursive per-device d_lock) for the whole tx drain; re-taking the
+   * same lock here guarantees that:
+   *
+   *   - a delaywork instance running on ETHWORK cannot seal and EP_SUBMIT the
+   *     NTB while cdcncm_send() is still formatting datagrams into wrreq->buf
+   *     on the net thread (dgramcount / dgramaddr / the buffer would tear),
+   *     and
+   *   - the synchronous buffer-full flush (called from cdcncm_send() under
+   *     this very lock -- a recursive re-acquire) can never overlap a
+   *     delaywork instance.
+   *
+   * Without this, delay-0 scheduling (the low-latency RTT path) lets two
+   * cdcncm_transmit_work() bodies run on different threads, EP_SUBMIT the
+   * single wrreq twice, corrupt the USB driver's IN request queue and leave
+   * the IN buffer prepared-but-unarmed (buffer-control shows LEN/PID but
+   * AVAILABLE clear) -- TX then wedges permanently (wrreq_idle is never
+   * reposted).  netdev_lock is a recursive nxrmutex, so the synchronous
+   * (already-locked) caller re-enters it safely.
+   */
+
+  netdev_lock(&self->dev.netdev);
+
+  /* Nothing to send means a previous flush (typically the synchronous
+   * buffer-full path in cdcncm_send) already emptied and submitted this
+   * batch.  Bail out rather than seal an empty NTB and EP_SUBMIT the still
+   * in-flight wrreq a second time.
    */
 
-  while (nxsem_wait(&self->wrreq_idle) != OK)
+  if (self->dgramcount == 0)
     {
+      netdev_unlock(&self->dev.netdev);
+      return;
     }
 
+  /* Ownership of the single wrreq->buf (the wrreq_idle token) has already
+   * been taken by cdcncm_send() when this NTB batch was started
+   * (self->dgramcount transitioned from 0), guaranteeing the previous
+   * transmission has completed.  Do not wait again here: the token is a
+   * counting semaphore initialised to 1, so a second wait would block
+   * forever (it is only reposted by cdcncm_wrcomplete after the EP_SUBMIT
+   * below).
+   */
+
   ncblen   = opts->nthsize;
   ndpindex = NCM_ALIGN(ncblen, ndpalign);
 
@@ -950,6 +987,8 @@
   self->wrreq->len = totallen;
 
   EP_SUBMIT(self->epbulkin, self->wrreq);
+
+  netdev_unlock(&self->dev.netdev);
 }
 
 /****************************************************************************
@@ -1291,6 +1330,31 @@
   FAR struct cdcncm_driver_s *self;
 
   self = container_of(dev, struct cdcncm_driver_s, dev);
+
+  /* Starting a new NTB batch?  The driver coalesces several datagrams into
+   * the single pre-allocated wrreq->buf.  That buffer may still be in-flight
+   * from the previous transmission (EP_SUBMIT'ed, wrcomplete not yet run):
+   * the USB controller transmits directly out of wrreq->buf, so we must not
+   * begin formatting new data into it until the hardware has finished with
+   * it.  Take ownership of the buffer here (waiting for the previous
+   * transfer to complete) instead of only just before EP_SUBMIT.
+   *
+   * This matters for TCP write buffers: a single txavail poll drains many
+   * queued segments back-to-back through cdcncm_send(), so a full NTB can be
+   * submitted and the next batch started while the first is still on the
+   * wire.  Without this guard the in-flight buffer gets overwritten, the
+   * host drops the corrupted NTB, and the transfer can wedge (wrreq_idle is
+   * never reposted), permanently stalling all TX.  The unbuffered send path
+   * transmits one segment per blocking send() and so never overlaps.
+   */
+
+  if (self->dgramcount == 0)
+    {
+      while (nxsem_wait(&self->wrreq_idle) != OK)
+        {
+        }
+    }
+
   cdcncm_transmit_format(self, pkt);
   netpkt_free(dev, pkt, NETPKT_TX);