[PATCH net-next 10/15] net: skbuff: reject a negative length in skb_copy_and_csum_bits()

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

Commit 417261779eab ("net: skbuff: don't BUG() on leftover length in
skb_copy_and_csum_bits()") guards its zero-fill against a negative @len
from a broken caller, but such a @len never gets that far.  The head,
frag and frag_list copies all clamp their length to @len, so a negative
one has already turned into a huge copy, or an endless frag walk, by
then.

Reject a negative @len on entry with -EINVAL, with a
DEBUG_NET_WARN_ON_ONCE() for debug kernels, and drop the guard on the
zero-fill that can no longer be reached with one.  Every caller already
handles an error from skb_copy_and_csum_bits().

Found by the Sashiko AI review of v2 of the series that made the
change.

Fixes: 417261779eab ("net: skbuff: don't BUG() on leftover length in 
skb_copy_and_csum_bits()")
Link: 
https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@xxxxxxxxxxxxxx/
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 net/core/skbuff.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 782a6bacd568..56cec58bccb7 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3678,9 +3678,10 @@ EXPORT_SYMBOL(skb_checksum);
  *     @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, and on -EFAULT the part of @to that couldn't be filled is
+ *     zeroed.
  */
 int skb_copy_and_csum_bits(const struct sk_buff *skb, int offset,
                           u8 *to, int len, __wsum *csump)
@@ -3692,6 +3693,14 @@ int skb_copy_and_csum_bits(const struct sk_buff *skb, 
int offset,
        __wsum csum = 0;
        int err;
 
+       /* Everything below clamps copies 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)
@@ -3777,8 +3786,7 @@ int skb_copy_and_csum_bits(const struct sk_buff *skb, int 
offset,
        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:

-- 
2.55.0