Re: kern/59943: usbnet(9) keeps failed packet in queue to retry indefinitely

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/59943; it has been noted by GNATS.

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: netbsd-bugs%NetBSD.org@localhost, gnats-bugs%NetBSD.org@localhost
Cc: mrg%NetBSD.org@localhost, skrll%NetBSD.org@localhost, jakllsch%NetBSD.org@localhost
Subject: Re: kern/59943: usbnet(9) keeps failed packet in queue to retry indefinitely
Date: Mon, 26 Jan 2026 03:53:26 +0000

 This is a multi-part message in MIME format.
 --=_La7nkjIJilZyzQ04m7iYdJZjSuvrF8DG
 
 The attached (untested) patch tries to address these issues (and some
 of those in PR kern/59940: usbnet(9): uno_tx_prepare buffer overrun
 audit); details in commit message.  Thoughts?
 
 --=_La7nkjIJilZyzQ04m7iYdJZjSuvrF8DG
 Content-Type: text/plain; charset="ISO-8859-1"; name="pr59940-pr59943-usbnettxfixes"
 Content-Transfer-Encoding: quoted-printable
 Content-Disposition: attachment; filename="pr59940-pr59943-usbnettxfixes.patch"
 
 # HG changeset patch
 # User Taylor R Campbell <riastradh%NetBSD.org@localhost>
 # Date 1769392552 0
 #      Mon Jan 26 01:55:52 2026 +0000
 # Branch trunk
 # Node ID f2a961ab939686586e96a4a42110282d83dcad30
 # Parent  ce5666ee3bee99fc43a60b487110d6418fb6dd5c
 # EXP-Topic riastradh-pr59940-usbnettxlenaudit
 usbnet(9): Tighten tx path.
 
 1. Verify, don't just assert, that the packet length is below the
    buffer size.  Even if all the callers enforce the interface's MTU,
    I can't prove that the usbnet(9) tx buffer size is an upper bound
    enforced on the interface's MTU.
 
    We can remove the check later if we do enforce that upper bound at
    some point, which would probably be worth doing anyway since the
    MTU is checked earlier in the tx path.
 
 2. Assert, don't check, that c->unc_xfer is nonnull.  We can only
    reach the tx path if we cross if_init=3Dusbnet_init_rx_tx, and that
    (via usbnet_tx_list_init) is guaranteed to fail and back out
    unless all of the usbnet_chain unc_xfers get initialized.
 
 3. If we can't fit the packet into a buffer, drop it -- don't leave
    it in the queue to try again when it still won't fit in the
    buffer.
 
 4. If the transfer for this packet fails, drop it -- don't leave it
    in the queue to try again just in case it might work better the
    next time.
 
 PR kern/59940: usbnet(9): uno_tx_prepare buffer overrun audit
 PR kern/59943: usbnet(9) keeps failed packet in queue to retry indefinitely
 
 diff -r ce5666ee3bee -r f2a961ab9396 sys/dev/usb/usbnet.c
 --- a/sys/dev/usb/usbnet.c	Sat Jan 24 23:11:52 2026 +0000
 +++ b/sys/dev/usb/usbnet.c	Mon Jan 26 01:55:52 2026 +0000
 @@ -521,26 +521,29 @@ usbnet_start_locked(struct ifnet *ifp)
  	idx =3D cd->uncd_tx_prod;
  	count =3D 0;
  	while (cd->uncd_tx_cnt < un->un_tx_list_cnt) {
 -		IFQ_POLL(&ifp->if_snd, m);
 +		IFQ_DEQUEUE(&ifp->if_snd, m);
  		if (m =3D=3D NULL) {
  			DPRINTF("start called, queue empty", 0, 0, 0, 0);
  			break;
  		}
 +		if (m->m_pkthdr.len > un->un_tx_bufsz) {
 +			DPRINTF("oversize packet, %ju > %ju",
 +			    m->m_pkthdr.len, un->un_tx_bufsz, 0, 0);
 +			if_statinc(ifp, if_oerrors);
 +			m_freem(m);
 +			continue;
 +		}
  		KASSERT(m->m_pkthdr.len <=3D un->un_tx_bufsz);
 =20
  		struct usbnet_chain *c =3D &cd->uncd_tx_chain[idx];
 +		KASSERT(c->unc_xfer !=3D NULL);
 =20
  		length =3D uno_tx_prepare(un, m, c);
  		if (length =3D=3D 0) {
  			DPRINTF("uno_tx_prepare gave zero length", 0, 0, 0, 0);
  			if_statinc(ifp, if_oerrors);
 -			break;
 -		}
 -
 -		if (__predict_false(c->unc_xfer =3D=3D NULL)) {
 -			DPRINTF("unc_xfer is NULL", 0, 0, 0, 0);
 -			if_statinc(ifp, if_oerrors);
 -			break;
 +			m_freem(m);
 +			continue;
  		}
 =20
  		usbd_setup_xfer(c->unc_xfer, c, c->unc_buf, length,
 @@ -552,12 +555,11 @@ usbnet_start_locked(struct ifnet *ifp)
  			DPRINTF("usbd_transfer on %#jx for %ju bytes: %jd",
  			    (uintptr_t)c->unc_buf, length, err, 0);
  			if_statinc(ifp, if_oerrors);
 -			break;
 +			m_freem(m);
 +			continue;
  		}
  		done_transmit =3D true;
 =20
 -		IFQ_DEQUEUE(&ifp->if_snd, m);
 -
  		/*
  		 * If there's a BPF listener, bounce a copy of this frame
  		 * to him.
 
 --=_La7nkjIJilZyzQ04m7iYdJZjSuvrF8DG--