Re: [PATCH] net: tulip: xircom_cb: drop runt frames before skb copy

[David Laight]

On Fri, 31 Jul 2026 20:33:34 +0200
Pablo Vallespín Aranguren <pablopva014@xxxxxxxxx> wrote:

> investigate_read_descriptor() takes the 11-bit packet length straight
> from the device-written descriptor and subtracts 4 for the CRC. A
> length below 4 underflows pkt_len to a negative value, which is then
> passed to skb_put_data() as a huge unsigned length and corrupts memory
> past the end of the freshly allocated skb.

Have you checked that that hardware can actually set a short value?
I'd expect that packets shorter than 64 bytes (including the crc) are
dropped as 'runts' and probably don't even use a ring entry.

Of course, if you think the device might lie all bets are off.
(Without an iommu is can write anywhere in host memory...)

	David

> 
> Drop the frame as a length error if pkt_len is negative after CRC
> removal.
> 
> Signed-off-by: Pablo Vallespín Aranguren <pablopva014@xxxxxxxxx>
> Assisted-by: gkh_clanker_t1000
> ---
>  drivers/net/ethernet/dec/tulip/xircom_cb.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/ethernet/dec/tulip/xircom_cb.c b/drivers/net/ethernet/dec/tulip/xircom_cb.c
> index e5d2ede13845..62c64da1c8fa 100644
> --- a/drivers/net/ethernet/dec/tulip/xircom_cb.c
> +++ b/drivers/net/ethernet/dec/tulip/xircom_cb.c
> @@ -1110,6 +1110,11 @@ investigate_read_descriptor(struct net_device *dev, struct xircom_private *card,
>  					/* minus 4, we don't want the CRC */
>  		struct sk_buff *skb;
>  
> +		if (pkt_len < 0) {
> +			dev->stats.rx_length_errors++;
> +			goto out;
> +		}
> +
>  		if (pkt_len > 1518) {
>  			netdev_err(dev, "Packet length %i is bogus\n", pkt_len);
>  			pkt_len = 1518;