Linux USB
 help / color / mirror / Atom feed
* [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS
@ 2026-11-10  9:03 Joas Antonio dos Santos via B4 Relay
  2026-11-10 22:41 ` Andrew Lunn
  2026-11-11  9:03 ` sashiko-bot
  0 siblings, 2 replies; 4+ messages in thread
From: Joas Antonio dos Santos via B4 Relay @ 2026-11-10  9:03 UTC (permalink / raw)
  To: joasantonio108, steve.glendinning, andrew+netdev, davem, edumazet,
	kuba, pabeni
  Cc: netdev, linux-usb, linux-kernel, stable

From: Joas Antonio dos Santos <joasantonio108@gmail.com>

smsc75xx_rx_fixup() takes the frame length from the RX command word
supplied by the device and strips the FCS by using "size - 4" for the
allocation, skb_put() and memcpy() of each frame that is not the last
one in the URB.  The length is only checked against an upper bound and
against skb->len, so a device can report a length field of 2..5, which
gives a size of 0..3 after RXW_PADDING is subtracted.

size is a u32, so "size - 4" then wraps to 0xfffffffc..0xffffffff.
__netdev_alloc_skb() adds NET_SKB_PAD to the unsigned length, which
wraps back to a small value and the allocation succeeds.  skb_put()
adds the length to the 32-bit skb->tail and skb->len, which also wrap,
so the skb_over_panic() check does not fire.  The following memcpy()
then tries to copy almost 4 GiB from the URB buffer into that small
allocation and writes far beyond it.

A malicious or malfunctioning USB device can use this to corrupt
kernel memory or crash the machine.

Reject frames that are too short to hold an FCS, as lan78xx_rx() does.

Found by LLM-assisted source review.

Fixes: 1b3b2d9e772b ("net: usb: smsc75xx: stop lying about skb->truesize")
Cc: stable@vger.kernel.org
Signed-off-by: Joas Antonio dos Santos <joasantonio108@gmail.com>
---
This issue was found with the help of an LLM-based code review tool
(Claude).  I checked the code paths by hand against v7.4-rc3 and
reviewed the fix myself.  It is build-tested only (W=1, arm64); I do
not have the hardware to test it at runtime.
---
 drivers/net/usb/smsc75xx.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
index 6f72a3f15907..e923fa6c1785 100644
--- a/drivers/net/usb/smsc75xx.c
+++ b/drivers/net/usb/smsc75xx.c
@@ -2217,6 +2217,13 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
 				return 0;
 			}
 
+			if (unlikely(size < ETH_FCS_LEN)) {
+				netif_dbg(dev, rx_err, dev->net,
+					  "size err rx_cmd_a=0x%08x\n",
+					  rx_cmd_a);
+				return 0;
+			}
+
 			/* last frame in this batch */
 			if (skb->len == size) {
 				smsc75xx_rx_csum_offload(dev, skb, rx_cmd_a,

---
base-commit: b3dfbde62cf1981c4638939c159dfd087b7a72e9
change-id: 20261110-smsc75xx-rx-fcs-fca968df1dce

Best regards,
-- 
Joas Antonio dos Santos <joasantonio108@gmail.com>



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS
  2026-11-10  9:03 [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS Joas Antonio dos Santos via B4 Relay
@ 2026-11-10 22:41 ` Andrew Lunn
  2026-11-11  2:00   ` Joas Antonio
  2026-11-11  9:03 ` sashiko-bot
  1 sibling, 1 reply; 4+ messages in thread
From: Andrew Lunn @ 2026-11-10 22:41 UTC (permalink / raw)
  To: joasantonio108
  Cc: steve.glendinning, andrew+netdev, davem, edumazet, kuba, pabeni,
	netdev, linux-usb, linux-kernel, stable

On Tue, Nov 10, 2026 at 09:03:21AM +0000, Joas Antonio dos Santos via B4 Relay wrote:
> From: Joas Antonio dos Santos <joasantonio108@gmail.com>
> 
> smsc75xx_rx_fixup() takes the frame length from the RX command word
> supplied by the device and strips the FCS by using "size - 4" for the
> allocation, skb_put() and memcpy() of each frame that is not the last
> one in the URB.  The length is only checked against an upper bound and
> against skb->len, so a device can report a length field of 2..5, which
> gives a size of 0..3 after RXW_PADDING is subtracted.
> 
> size is a u32, so "size - 4" then wraps to 0xfffffffc..0xffffffff.
> __netdev_alloc_skb() adds NET_SKB_PAD to the unsigned length, which
> wraps back to a small value and the allocation succeeds.  skb_put()
> adds the length to the 32-bit skb->tail and skb->len, which also wrap,
> so the skb_over_panic() check does not fire.  The following memcpy()
> then tries to copy almost 4 GiB from the URB buffer into that small
> allocation and writes far beyond it.
> 
> A malicious or malfunctioning USB device can use this to corrupt
> kernel memory or crash the machine.
> 
> Reject frames that are too short to hold an FCS, as lan78xx_rx() does.

I assume there is soon going to be another check to ensure the frame
is big enough to have an ethernet header? Can these checks be
combined?

	Andrew

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS
  2026-11-10 22:41 ` Andrew Lunn
@ 2026-11-11  2:00   ` Joas Antonio
  0 siblings, 0 replies; 4+ messages in thread
From: Joas Antonio @ 2026-11-11  2:00 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: steve.glendinning, andrew+netdev, davem, edumazet, kuba, pabeni,
	netdev, linux-usb, linux-kernel, stable

> I assume there is soon going to be another check to ensure the frame
> is big enough to have an ethernet header? Can these checks be
> combined?

Yes, that makes sense, thanks.

Looking at it again, short frames are only half covered today.  For
the last frame in the URB usbnet drops anything shorter than ETH_HLEN
after rx_fixup() returns.  The other frames are copied and passed
directly to usbnet_skb_return(), so a frame of 4..17 bytes (0..13
after the FCS is stripped) reaches eth_type_trans() without a complete
Ethernet header.

I will send a v2 that tests for size < ETH_HLEN + ETH_FCS_LEN in the
one place, which covers both the underflow and the missing header.

Regards,
Joas

Em ter., 10 de nov. de 2026 às 19:41, Andrew Lunn <andrew@lunn.ch> escreveu:
>
> On Tue, Nov 10, 2026 at 09:03:21AM +0000, Joas Antonio dos Santos via B4 Relay wrote:
> > From: Joas Antonio dos Santos <joasantonio108@gmail.com>
> >
> > smsc75xx_rx_fixup() takes the frame length from the RX command word
> > supplied by the device and strips the FCS by using "size - 4" for the
> > allocation, skb_put() and memcpy() of each frame that is not the last
> > one in the URB.  The length is only checked against an upper bound and
> > against skb->len, so a device can report a length field of 2..5, which
> > gives a size of 0..3 after RXW_PADDING is subtracted.
> >
> > size is a u32, so "size - 4" then wraps to 0xfffffffc..0xffffffff.
> > __netdev_alloc_skb() adds NET_SKB_PAD to the unsigned length, which
> > wraps back to a small value and the allocation succeeds.  skb_put()
> > adds the length to the 32-bit skb->tail and skb->len, which also wrap,
> > so the skb_over_panic() check does not fire.  The following memcpy()
> > then tries to copy almost 4 GiB from the URB buffer into that small
> > allocation and writes far beyond it.
> >
> > A malicious or malfunctioning USB device can use this to corrupt
> > kernel memory or crash the machine.
> >
> > Reject frames that are too short to hold an FCS, as lan78xx_rx() does.
>
> I assume there is soon going to be another check to ensure the frame
> is big enough to have an ethernet header? Can these checks be
> combined?
>
>         Andrew



-- 
Joas Antonio dos Santos
Cyber Security Specialist and Ethical Hacker
joasantonio108@gmail.com
LinkedIn

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS
  2026-11-10  9:03 [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS Joas Antonio dos Santos via B4 Relay
  2026-11-10 22:41 ` Andrew Lunn
@ 2026-11-11  9:03 ` sashiko-bot
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-11-11  9:03 UTC (permalink / raw)
  To: Joas Antonio dos Santos; +Cc: linux-usb

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] net: usb: smsc75xx: short non-final frames in
  smsc75xx_rx_fixup() read uninit memory
--

commit c9531c5cff866a425adb994e4854a216487a72e0
Author: Joas Antonio dos Santos <joasantonio108@gmail.com>

net: usb: smsc75xx: reject frames shorter than the FCS

This commit adds a minimum frame size check in smsc75xx_rx_fixup() to
reject frames shorter than ETH_FCS_LEN before subtracting 4 for the FCS.

> diff --git a/drivers/net/usb/smsc75xx.c b/drivers/net/usb/smsc75xx.c
> index 6f72a3f159079..e923fa6c17857 100644
> --- a/drivers/net/usb/smsc75xx.c
> +++ b/drivers/net/usb/smsc75xx.c
> @@ -2217,6 +2217,13 @@ static int smsc75xx_rx_fixup(struct usbnet *dev, struct sk_buff *skb)
>  				return 0;
>  			}
>  
> +			if (unlikely(size < ETH_FCS_LEN)) {
> +				netif_dbg(dev, rx_err, dev->net,
> +					  "size err rx_cmd_a=0x%08x\n",
> +					  rx_cmd_a);
> +				return 0;
> +			}
> +
>  			/* last frame in this batch */
>  			if (skb->len == size) {
>  				smsc75xx_rx_csum_offload(dev, skb, rx_cmd_a,

[Severity: Medium]
Should this check for size < ETH_HLEN + ETH_FCS_LEN instead of
size < ETH_FCS_LEN?

For a non-final frame in a batch (skb->len != size) with size in 4..17,
smsc75xx_rx_fixup() copies only 0..13 bytes (size - 4) into ax_skb->data
and passes ax_skb directly to usbnet_skb_return().  Unlike rx_process(),
which checks skb->len < ETH_HLEN for the final frame, usbnet_skb_return()
calls eth_type_trans() unconditionally, where eth_skb_pkt_type() and
eth_type_trans() read eth->h_dest and eth->h_proto from uninitialized
memory past the copied bytes in ax_skb->data.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-smsc75xx-rx-fcs-v1-1-4e048bfc28f3@gmail.com?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-11-11  9:03 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-10  9:03 [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS Joas Antonio dos Santos via B4 Relay
2026-11-10 22:41 ` Andrew Lunn
2026-11-11  2:00   ` Joas Antonio
2026-11-11  9:03 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox