Re: [PATCH net-next 1/9] net: skbuff: don't leave stale bytes in skb_copy_and_csum_bits()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] skbuff: skb_copy_and_csum_bits() discards linear part csum on unreadable frags
--

commit 554d4ec9904f25474f32c877a62ca154d0b20324
Author: Josef Bacik <josef@toxicpanda.com>

net: skbuff: don't leave stale bytes in skb_copy_and_csum_bits()

This commit zeroes out the remainder of a caller's buffer when
skb_copy_and_csum_bits encounters unreadable fragments. This prevents
stale uninitialized memory from being leaked when the buffer is
transmitted on the wire.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 5c4024a03e105..512ff9cfa269f 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3633,8 +3633,12 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
>  		pos	= copy;
>  	}
>  
> -	if (!skb_frags_readable(skb))
> +	if (!skb_frags_readable(skb)) {
> +		/* Don't hand the caller a buffer with stale bytes in it. */
> +		if (len > 0)
> +			memset(to, 0, len);
>  		return 0;

[Severity: Medium]
By returning 0 here instead of the accumulated csum, does this discard
the checksum computed for the successfully copied linear part of the packet?

When icmp_push_reply() uses skb_copy_and_csum_bits() via icmp_glue_bits()
to quote an offending packet with unreadable fragments, the offending
packet typically has its headers in the linear part. Returning 0 discards
this partial checksum.

Since the ICMP payload contains the non-zero linear part, returning 0
causes the ICMP packet's overall checksum to be computed incorrectly,
resulting in the error packet being silently dropped by the destination.

Would it be better to return csum here instead of 0, so that the checksum
is mathematically correct for the zero-padded buffer and allows the ICMP
error to be successfully delivered?

> +	}
>  
>  	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
>  		int end;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-b4-skbuff-bug-on-v1-0-1b4434c5357c@toxicpanda.com?part=1