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

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

[Tianchu Chen]

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.

Return immediately after scheduling the recovery work so the marker skb
is dropped instead of being assembled as packet data.

Fixes: ca139d76b0d9 ("cx82310_eth: re-enable ethernet mode after router reboot")
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
 drivers/net/usb/cx82310_eth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/cx82310_eth.c b/drivers/net/usb/cx82310_eth.c
index 068acb052..5df657acf 100644
--- a/drivers/net/usb/cx82310_eth.c
+++ b/drivers/net/usb/cx82310_eth.c
@@ -282,6 +282,7 @@ static int cx82310_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 		if (len == 0xffff) {
 			netdev_info(dev->net, "router was rebooted, re-enabling ethernet mode");
 			schedule_work(&priv->reenable_work);
+			return 0;
 		} else if (len > CX82310_MTU) {
 			netdev_err(dev->net, "RX packet too long: %d B\n", len);
 			return 0;
-- 
2.51.0

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

[Jakub Kicinski]

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

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

[Tianchu Chen]

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 */
...
...
	}

For normal oversized lengths, the len > CX82310_MTU branch rejects
before this copy. But 0xffff is special-cased and falls through. With a
4096-byte RX URB, after the 2-byte length header is pulled, skb->len can
be 4094, while partial_data is allocated as dev->hard_mtu
(CX82310_MTU + 2, 1516 bytes).

So the source read is bounded by skb->len; the destination write is not.

I am happy to send a v2 with this spelled out more clearly in the commit message
if needed.

Best regards,
Tianchu Chen

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

[Jakub Kicinski]

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?

> ...
> ...
> 	}
> 
> For normal oversized lengths, the len > CX82310_MTU branch rejects
> before this copy. But 0xffff is special-cased and falls through. With a
> 4096-byte RX URB, after the 2-byte length header is pulled, skb->len can
> be 4094, while partial_data is allocated as dev->hard_mtu
> (CX82310_MTU + 2, 1516 bytes).
> 
> So the source read is bounded by skb->len; the destination write is not.
> 
> I am happy to send a v2 with this spelled out more clearly in the commit message
> if needed.
> 
> Best regards,
> Tianchu Chen
> 

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

[Tianchu Chen]

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