Re: [PATCH net-next v2 6/8] 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:
- [Medium] skb: negative len from broken caller causes OOB write before memset guard
--

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

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

This commit removes a BUG_ON(len) check and instead returns 0 after zeroing
any unfilled buffer space. It explicitly aims to avoid a massive memset if
a broken caller passes a negative length.

> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index c896770203f71..7fd2f8142cc41 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3713,7 +3713,13 @@ __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
>  		}
>  		start = end;
>  	}
> -	BUG_ON(len);
> +	if (unlikely(len)) {

[Severity: Medium]
Will this check ever be reached if a caller passes a negative len?

The commit message mentions avoiding a huge memset if len is negative,
but if len is negative, earlier loops in skb_copy_and_csum_bits() will
clamp their copy size to this negative value:

net/core/skbuff.c:skb_copy_and_csum_bits() {
    ...
    if (copy > 0) {
        if (copy > len)
            copy = len;
        csum = csum_partial_copy_nocheck(skb->data + offset, to, copy);
    ...
}

This negative copy value causes csum_partial_copy_nocheck() to perform a
massive out-of-bounds access.

Similarly, in the fragment loop, a negative len makes copy negative,
leading to an infinite loop and out-of-bounds writes in
skb_frag_foreach_page() due to unsigned comparisons.

Does this function need an early return for negative lengths at the entry
point to truly protect against broken callers?

> +		DEBUG_NET_WARN_ON_ONCE(1);
> +		/* Don't hand the caller a buffer with stale bytes in it. */
> +		if (len > 0)
> +			memset(to, 0, len);
> +		return 0;
> +	}
>  	return csum;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@toxicpanda.com?part=6