[PATCH net v2 1/2] net: usb: pegasus: do not advance rx_skb->data on dropped frames

[PATCH net v2 1/2] net: usb: pegasus: do not advance rx_skb->data on dropped frames

[Wei Jie Law]

For the 0x8513 chip, read_bulk_callback() skips the 2-byte receive
header by advancing pegasus->rx_skb->data in place. This is done before
the pkt_len > PEGASUS_MTU check, and when that check drops the frame
the goon path resubmits the same skb with the advanced rx_skb->data as
the transfer buffer, still PEGASUS_MTU bytes long.

Every frame with an oversized length field therefore moves the receive
buffer two more bytes towards the end of the skb data area. A device
that keeps sending such frames makes the host controller write received
data over skb_shared_info and past the end of the allocation, and the
corrupted skb_shared_info is used when the skb is eventually freed.
With an emulated device sending frames filled with 0x5a on a KASAN
kernel:

  BUG: KASAN: slab-out-of-bounds in dummy_timer+0xade/0x3f00 [dummy_hcd]
  Write of size 1536 at addr ffff888105821202 by task swapper/1/0
  ...
  Oops: general protection fault, probably for non-canonical address 0xeb4b474b4b4b4b4c: 0000 [#1] PREEMPT SMP KASAN NOPTI
  KASAN: maybe wild-memory-access in range [0x5a5a5a5a5a5a5a60-0x5a5a5a5a5a5a5a67]
  RIP: 0010:skb_release_data+0x36f/0x800

Only skip the header with skb_reserve() once the frame is going to be
passed up, so the receive buffer always starts at the beginning of the
skb data area. Unlike the old pointer adjustment, skb_reserve() also
moves skb->tail, so the two header bytes now count against the buffer
size: lower the length limit for the 0x8513 accordingly, otherwise a
pkt_len of PEGASUS_MTU - 1 or PEGASUS_MTU would trip skb_over_panic().

Found by code review with LLM assistance and reproduced with an
emulated USB device (raw-gadget + dummy_hcd).

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>
---
 drivers/net/usb/pegasus.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index aba1a640fc26..7c7d6b69998f 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -461,6 +461,7 @@ static void read_bulk_callback(struct urb *urb)
 	u8 *buf = urb->transfer_buffer;
 	int rx_status, count = urb->actual_length;
 	int status = urb->status;
+	int rx_offset = 0;
 	__u16 pkt_len;
 
 	if (!pegasus)
@@ -510,7 +511,7 @@ static void read_bulk_callback(struct urb *urb)
 	if (pegasus->chip == 0x8513) {
 		pkt_len = le32_to_cpu(*(__le32 *)urb->transfer_buffer);
 		pkt_len &= 0x0fff;
-		pegasus->rx_skb->data += 2;
+		rx_offset = 2;
 	} else {
 		pkt_len = buf[count - 3] << 8;
 		pkt_len += buf[count - 4];
@@ -522,13 +523,14 @@ static void read_bulk_callback(struct urb *urb)
 	 * If the packet is unreasonably long, quietly drop it rather than
 	 * kernel panicing by calling skb_put.
 	 */
-	if (pkt_len > PEGASUS_MTU)
+	if (pkt_len > PEGASUS_MTU - rx_offset)
 		goto goon;
 
 	/*
 	 * at this point we are sure pegasus->rx_skb != NULL
 	 * so we go ahead and pass up the packet.
 	 */
+	skb_reserve(pegasus->rx_skb, rx_offset);
 	skb_put(pegasus->rx_skb, pkt_len);
 	pegasus->rx_skb->protocol = eth_type_trans(pegasus->rx_skb, net);
 	netif_rx(pegasus->rx_skb);
-- 
2.43.0

Re: [PATCH net v2 1/2] net: usb: pegasus: do not advance rx_skb->data on dropped frames

[Simon Horman]

On Wed, Oct 21, 2026 at 10:03:09AM +0800, Wei Jie Law wrote:
> For the 0x8513 chip, read_bulk_callback() skips the 2-byte receive
> header by advancing pegasus->rx_skb->data in place. This is done before
> the pkt_len > PEGASUS_MTU check, and when that check drops the frame
> the goon path resubmits the same skb with the advanced rx_skb->data as
> the transfer buffer, still PEGASUS_MTU bytes long.
> 
> Every frame with an oversized length field therefore moves the receive
> buffer two more bytes towards the end of the skb data area. A device
> that keeps sending such frames makes the host controller write received
> data over skb_shared_info and past the end of the allocation, and the
> corrupted skb_shared_info is used when the skb is eventually freed.
> With an emulated device sending frames filled with 0x5a on a KASAN
> kernel:
> 
>   BUG: KASAN: slab-out-of-bounds in dummy_timer+0xade/0x3f00 [dummy_hcd]
>   Write of size 1536 at addr ffff888105821202 by task swapper/1/0
>   ...
>   Oops: general protection fault, probably for non-canonical address 0xeb4b474b4b4b4b4c: 0000 [#1] PREEMPT SMP KASAN NOPTI
>   KASAN: maybe wild-memory-access in range [0x5a5a5a5a5a5a5a60-0x5a5a5a5a5a5a5a67]
>   RIP: 0010:skb_release_data+0x36f/0x800
> 
> Only skip the header with skb_reserve() once the frame is going to be
> passed up, so the receive buffer always starts at the beginning of the
> skb data area. Unlike the old pointer adjustment, skb_reserve() also
> moves skb->tail, so the two header bytes now count against the buffer
> size: lower the length limit for the 0x8513 accordingly, otherwise a
> pkt_len of PEGASUS_MTU - 1 or PEGASUS_MTU would trip skb_over_panic().
> 
> Found by code review with LLM assistance and reproduced with an
> emulated USB device (raw-gadget + dummy_hcd).
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>

Reviewed-by: Simon Horman <horms@kernel.org>

[PATCH net v2 2/2] net: usb: pegasus: check rx pkt_len against the received length

[Wei Jie Law]

read_bulk_callback() takes the frame length from the device, from the
receive header on the 0x8513 and from the trailing status bytes on the
other chips, and only checks it against PEGASUS_MTU before passing that
many bytes up the stack. In two cases the stack then uses bytes that the
device did not send:

- A device that sends a short transfer but claims a long frame gets the
  stale or uninitialized remainder of the receive skb delivered as
  packet data.

- A device that claims a frame shorter than an Ethernet header gets
  past the check as well. eth_type_trans() takes pkt_type from h_dest
  and the protocol from h_proto without checking skb->len. If the
  transfer is also short, those come from bytes left in the skb by an
  earlier transfer, or from uninitialized memory.

Drop frames shorter than ETH_HLEN, and frames whose claimed length, plus
the 0x8513 header, does not fit in the bytes actually received, and
count them as length errors. urb->actual_length never exceeds the
PEGASUS_MTU sized transfer buffer, so this also covers the existing
PEGASUS_MTU check. Valid Ethernet frames are at least 60 bytes long
without the FCS, so they are not affected.

Found by code review with LLM assistance. With an emulated USB device,
a 101-byte transfer claiming a 1500-byte frame was passed up as a
1500-byte packet before this change and is dropped after it, and
frames claiming 0, 4 and 13 bytes were passed up and are now dropped
and counted in rx_length_errors. The extra bytes happened to be zero
in the first test, so disclosure of actual kernel data was not
demonstrated.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>
---
 drivers/net/usb/pegasus.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/net/usb/pegasus.c b/drivers/net/usb/pegasus.c
index 7c7d6b69998f..68e4b6658dd2 100644
--- a/drivers/net/usb/pegasus.c
+++ b/drivers/net/usb/pegasus.c
@@ -520,11 +520,17 @@ static void read_bulk_callback(struct urb *urb)
 	}
 
 	/*
-	 * If the packet is unreasonably long, quietly drop it rather than
-	 * kernel panicing by calling skb_put.
+	 * Drop frames shorter than an Ethernet header, because
+	 * eth_type_trans() reads the header whatever the length, and frames
+	 * longer than what was actually received. Passing either up would
+	 * expose stale buffer contents, and skb_put() could panic on the
+	 * latter. count is at most PEGASUS_MTU.
 	 */
-	if (pkt_len > PEGASUS_MTU - rx_offset)
+	if (pkt_len < ETH_HLEN || pkt_len + rx_offset > count) {
+		net->stats.rx_errors++;
+		net->stats.rx_length_errors++;
 		goto goon;
+	}
 
 	/*
 	 * at this point we are sure pegasus->rx_skb != NULL
-- 
2.43.0

Re: [PATCH net v2 2/2] net: usb: pegasus: check rx pkt_len against the received length

[Simon Horman]

On Wed, Oct 21, 2026 at 10:03:10AM +0800, Wei Jie Law wrote:
> read_bulk_callback() takes the frame length from the device, from the
> receive header on the 0x8513 and from the trailing status bytes on the
> other chips, and only checks it against PEGASUS_MTU before passing that
> many bytes up the stack. In two cases the stack then uses bytes that the
> device did not send:
> 
> - A device that sends a short transfer but claims a long frame gets the
>   stale or uninitialized remainder of the receive skb delivered as
>   packet data.
> 
> - A device that claims a frame shorter than an Ethernet header gets
>   past the check as well. eth_type_trans() takes pkt_type from h_dest
>   and the protocol from h_proto without checking skb->len. If the
>   transfer is also short, those come from bytes left in the skb by an
>   earlier transfer, or from uninitialized memory.
> 
> Drop frames shorter than ETH_HLEN, and frames whose claimed length, plus
> the 0x8513 header, does not fit in the bytes actually received, and
> count them as length errors. urb->actual_length never exceeds the
> PEGASUS_MTU sized transfer buffer, so this also covers the existing
> PEGASUS_MTU check. Valid Ethernet frames are at least 60 bytes long
> without the FCS, so they are not affected.
> 
> Found by code review with LLM assistance. With an emulated USB device,
> a 101-byte transfer claiming a 1500-byte frame was passed up as a
> 1500-byte packet before this change and is dropped after it, and
> frames claiming 0, 4 and 13 bytes were passed up and are now dropped
> and counted in rx_length_errors. The extra bytes happened to be zero
> in the first test, so disclosure of actual kernel data was not
> demonstrated.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Wei Jie Law <law.wei.jie@gt.tech.gov.sg>

Reviewed-by: Simon Horman <horms@kernel.org>