The head, frag and frag_list copies in skb_copy_and_csum_bits() each clamp their length to @len. A negative @len from a broken caller therefore becomes the copy length, and the copy takes it as a huge unsigned value. The two len > 0 tests around the zero-fills only protect the zero-fills. Reject a negative @len on entry with -EINVAL and drop the two tests. @to isn't touched in that case. Every caller already handles an error, and skb_copy_and_csum_dev() zeroes its whole frame on one. No caller is known to pass a negative length. Found by the Sashiko AI review of the BUG_ON() series. Link: https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@toxicpanda.com/ Signed-off-by: Josef Bacik --- net/core/skbuff.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/net/core/skbuff.c b/net/core/skbuff.c index b74d8a6ef0f9..e3fc6291d223 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -3731,8 +3731,7 @@ static noinline int skb_copy_and_csum_frag_list(const struct sk_buff *skb, if (unlikely(len)) { 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); + memset(to, 0, len); return -EFAULT; } out: @@ -3748,9 +3747,10 @@ static noinline int skb_copy_and_csum_frag_list(const struct sk_buff *skb, * @len: number of bytes to copy * @csump: where to store the checksum of the copied bytes * - * Return: 0 on success, or -EFAULT if @skb has unreadable frags or runs - * out before @len does. On failure *@csump is not set and the part of - * @to that couldn't be filled is zeroed. + * Return: 0 on success, -EINVAL if @len is negative, or -EFAULT if @skb + * has unreadable frags or runs out before @len does. On failure *@csump + * is not set. On -EFAULT the part of @to that couldn't be filled is + * zeroed, and on -EINVAL @to is not touched. */ int skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, u8 *to, int len, __wsum *csump) @@ -3760,6 +3760,14 @@ int skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, int pos = 0; __wsum csum = 0; + /* Everything below clamps its copy to @len, so a negative one would + * turn into a huge copy. + */ + if (unlikely(len < 0)) { + DEBUG_NET_WARN_ON_ONCE(1); + return -EINVAL; + } + /* Copy header. */ if (copy > 0) { if (copy > len) @@ -3775,8 +3783,7 @@ int skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, if (!skb_frags_readable(skb)) { /* Don't hand the caller a buffer with stale bytes in it. */ - if (len > 0) - memset(to, 0, len); + memset(to, 0, len); return -EFAULT; } -- 2.55.0