git: 40ad87e958e8 - main - usb: if_ure: stop touching the mbuf accounting on rxq insertion

From: Kyle Evans <kevans_at_FreeBSD.org>
Date: Sun, 20 Apr 2025 18:28:20 UTC
The branch main has been updated by kevans:

URL: https://cgit.FreeBSD.org/src/commit/?id=40ad87e958e8a9cd5d5c3380e709c7503dd12210

commit 40ad87e958e8a9cd5d5c3380e709c7503dd12210
Author:     Kyle Evans <kevans@FreeBSD.org>
AuthorDate: 2025-04-20 18:28:12 +0000
Commit:     Kyle Evans <kevans@FreeBSD.org>
CommitDate: 2025-04-20 18:28:12 +0000

    usb: if_ure: stop touching the mbuf accounting on rxq insertion
    
    uether_rxmbuf() assumes the uether_newbuf() model where the caller has
    just set m_len to the entire size of the mbuf, and passed the final
    size into uether_rxmbuf() for finalization.
    
    In if_ure we're creating our own mbuf chains, and the accounting is all
    already accurate.  uether_rxmbuf's logic won't work at all for a chain,
    so let's add an assertion to that effect (but I think the other callers
    were all OK).
    
    This fixes a panic on my Windows DevKit when undergoing any kind of
    network stress that surfaces after the bogus mbuf is injected into the
    network stack (usually observed in `m_dup`).
    
    markj and I had spent some time investigating it and decided there's
    some kind of buffer underrun happening; the rx packet descriptor reports
    a length longer than what's actually available.  We discard the rest of
    the transfer, but then we end up fetching it in a subsequent transfer
    and end up casting packet data to a `struct ure_rxpkt` incorrectly.  It
    would be better to fix the underlying root cause, but this is a
    reasonable mitigation in the interim.
    
    Reviewed by:    markj
    Fixes:  7d5522e16a ("A major update to the ure driver.")
    Differential Revision:  https://reviews.freebsd.org/D43465
---
 sys/dev/usb/net/if_ure.c       | 8 +++++++-
 sys/dev/usb/net/usb_ethernet.c | 9 ++++++++-
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/sys/dev/usb/net/if_ure.c b/sys/dev/usb/net/if_ure.c
index 7c9b74334b58..c3f7b622d687 100644
--- a/sys/dev/usb/net/if_ure.c
+++ b/sys/dev/usb/net/if_ure.c
@@ -707,7 +707,13 @@ ure_bulk_read_callback(struct usb_xfer *xfer, usb_error_t error)
 				/* set the necessary flags for rx checksum */
 				ure_rxcsum(caps, &pkt, m);
 
-				uether_rxmbuf(ue, m, len - ETHER_CRC_LEN);
+				/*
+				 * len has been known to be bogus at times,
+				 * which leads to problems when passed to
+				 * uether_rxmbuf().  Better understanding why we
+				 * can get there make for good future work.
+				 */
+				uether_rxmbuf(ue, m, 0);
 			}
 
 			off += roundup(len, URE_RXPKT_ALIGN);
diff --git a/sys/dev/usb/net/usb_ethernet.c b/sys/dev/usb/net/usb_ethernet.c
index 977805cefe66..692ea64128b9 100644
--- a/sys/dev/usb/net/usb_ethernet.c
+++ b/sys/dev/usb/net/usb_ethernet.c
@@ -593,7 +593,14 @@ uether_rxmbuf(struct usb_ether *ue, struct mbuf *m,
 	/* finalize mbuf */
 	if_inc_counter(ifp, IFCOUNTER_IPACKETS, 1);
 	m->m_pkthdr.rcvif = ifp;
-	m->m_pkthdr.len = m->m_len = len;
+	if (len != 0) {
+		/*
+		 * This is going to get it wrong for an mbuf chain, so let's
+		 * make sure we're not doing that.
+		 */
+		MPASS(m->m_next == NULL);
+		m->m_pkthdr.len = m->m_len = len;
+	}
 
 	/* enqueue for later when the lock can be released */
 	(void)mbufq_enqueue(&ue->ue_rxq, m);