Re: [PATCH net-next 7/9] net: skbuff: don't BUG() on leftover length 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:
- [High] net: incomplete defense against negative len in skb_copy_and_csum_bits
--

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

net: skbuff: don't BUG() on leftover length in skb_copy_and_csum_bits()

This commit replaces a BUG_ON(len) with a WARN_ON_ONCE(len) at the end of
skb_copy_and_csum_bits(). It also clears any remaining space in the destination
buffer if len is positive, preventing stale bytes from being sent.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index c8c2c0319b87a..e29eda2eaf3f6 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3709,7 +3709,12 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
>  		}
>  		start = end;
>  	}
> -	BUG_ON(len);
> +	if (WARN_ON_ONCE(len)) {
> +		/* Don't hand the caller a buffer with stale bytes in it. */
> +		if (len > 0)
> +			memset(to, 0, len);
> +		return 0;
> +	}

[Severity: High]
The commit message states that "a negative @len from a broken caller must not
turn into a huge memset()". If a broken caller passes a negative len, will we
actually reach this check, or will it cause memory corruption earlier in the
function?

Earlier in skb_copy_and_csum_bits(), if offset is smaller than
skb_headlen(skb), copy starts as positive. If a broken caller passes a
negative len, copy > len evaluates to true, forcing copy = len:

	if (copy > 0) {
		if (copy > len)
			copy = len;
		csum = csum_partial_copy_nocheck(skb->data + offset, to,
						 copy);

Could this pass the negative copy to csum_partial_copy_nocheck(), which
interprets it as a huge unsigned size and causes a massive out-of-bounds
read and write before ever reaching the new guard?

>  	return csum;
>  }
>  EXPORT_SYMBOL(skb_copy_and_csum_bits);

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