* [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