Re: [PATCH] net: usb: cx82310_eth: stop parsing reboot marker as packet

["Tianchu Chen" <tianchu.chen@linux.dev>]

July 1, 2026 at 6:42 AM, "Jakub Kicinski" <kuba@kernel.org mailto:kuba@kernel.org?to=%22Jakub%20Kicinski%22%20%3Ckuba%40kernel.org%3E > wrote:

> 
> On Tue, 30 Jun 2026 10:30:53 +0000 Tianchu Chen wrote:
> 
> > 
> > June 30, 2026 at 8:44 AM, "Jakub Kicinski" <kuba@kernel.org mailto:kuba@kernel.org?to=%22Jakub%20Kicinski%22%20%3Ckuba%40kernel.org%3E > wrote:
> >  On Thu, 25 Jun 2026 15:32:04 +0000 Tianchu Chen wrote:
> >  > From: Tianchu Chen <flynnnchen@tencent.com>
> >  > 
> >  > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> >  > 
> >  > cx82310_rx_fixup() treats an RX length of 0xffff as a device reboot
> >  > marker and schedules work to re-enable ethernet mode, but then continues
> >  > processing the marker as a normal packet length. This is an out-of-bounds
> >  > heap write controlled by the usb device.
> >  > 
> >  Where? Can you be more specific in the commit message? At a glance 
> >  the accesses seem to be bound-checked with skb->len.
> >  -- 
> >  pw-bot: cr
> >  
> >  
> >  
> >  The "len > skb->len" check bounds the source read, but the overflow is on the
> >  destination buffer.
> >  
> >  The buggy path is:
> >  
> >  if (len == 0xffff) {
> >  netdev_info(dev->net, "router was rebooted, re-enabling ethernet mode");
> >  schedule_work(&priv->reenable_work);
> >  /* <- BUG: missing return; 0xffff bypasses the oversized-length reject */
> >  } else if (len > CX82310_MTU) {
> >  netdev_err(dev->net, "RX packet too long: %d B\n", len);
> >  return 0;
> >  }
> >  if (len > skb->len) {
> >  dev->partial_len = skb->len; // skb->len is bounded by the USB transfer size (4K)
> >  dev->partial_rem = len - skb->len;
> >  memcpy((void *)dev->partial_data, skb->data,
> >  dev->partial_len); /* <- TRIGGER: can copy 4K bytes into 1516-byte partial_data */
> > 
> If skb->len (== dev->partial_len) is not bound-checked to the size
> of dev->partial_data - aren't there more paths that could hit this
> overflow? Are you fixing the right thing?

Yes, skb->len and CX82310_MTU are different limits here.

skb->len is the amount of data received in the current USB RX URB. This
driver sets dev->rx_urb_size to 4096, so skb->len can be much larger than
the network frame MTU.

The safety invariant for the partial_data copy is not that skb->len is
MTU-bounded by itself. It is that, for normal frames, the code first rejects
len > CX82310_MTU, and then reaches the partial-packet path only when
len > skb->len. Therefore, on the normal path:

	skb->len < len <= CX82310_MTU

so copying skb->len bytes into partial_data is safe, since partial_data is
allocated as dev->hard_mtu = CX82310_MTU + 2.

The 0xffff reboot marker is the only case that breaks that invariant:
len == 0xffff is handled in a separate branch, and it just bypasses the
"len > CX82310_MTU" check entirely.

So the only case that may trigger this OOB-write is len == 0xffff, which is a
reboot signal and should be skipped from being parsed as normal packet.

Also, skb->len is governed by the USB RX URB size, not by the network MTU;
it is a different length limit from CX82310_MTU.

I believe this addresses the concern, but please let me know if you see any
remaining issue and/or would prefer a v2.


Best regards,
Tianchu