Re: [PATCH net] net: usb: smsc75xx: reject frames shorter than the FCS

[sashiko-bot@kernel.org]

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