Thread (38 messages) 38 messages, 4 authors, 5d ago

Re: [PATCH net-next 03/15] net: skbuff: fail checksum validation when the skb can't be checksummed

flat view

From: Mina Almasry <hidden>
Date: 2026-10-23 14:37:37
Also in: xen-devel

On Mon, Oct 19, 2026 at 6:51 AM Josef Bacik [off-list ref] wrote:
quoted hunk ↗ jump to hunk
__skb_checksum_complete() verifies a received packet with
skb_checksum(), which returns 0 when the range runs into unreadable
frags or past the end of the skb.  0 is a valid sum, so it's treated as
one: it adds skb->csum, the pseudo-header sum, and folds the result.
With a sum of 0 that's csum_fold(skb->csum), which is 0 whenever the
pseudo-header sum folds to 0xffff, so a packet whose payload was never
checksummed is accepted, marked csum_valid, and left with a wrong
CHECKSUM_COMPLETE value in skb->csum.

__skb_checksum_complete_head() passes skb->csum in as the seed, so a
failure there folds 0 and the packet fails validation, but only because
skb_checksum() happened to drop the seed.

Use skb_checksum_full() in both, and report a packet that can't be
checksummed as having a bad checksum, so callers drop it the way they
drop any other bad checksum.  Clear csum_valid, and leave skb->csum and
ip_summed alone, since there's no full packet checksum to store.

Found by the Sashiko AI review of the series that made skb_checksum()
return 0 for a range past the end of the skb.

Fixes: 20ddd0fd2fcd ("net: skbuff: don't BUG() on leftover length in skb_checksum() and friends")
Link: https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@toxicpanda.com/ (local)
Assisted-by: LLM
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 net/core/skbuff.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 061c61422a38..40d2dd20d47d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3860,9 +3860,17 @@ EXPORT_SYMBOL(skb_crc32c);

 __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
 {
+       __wsum csum = skb->csum;
        __sum16 sum;

-       sum = csum_fold(skb_checksum(skb, 0, len, skb->csum));
+       /* If the data can't be checksummed it can't be valid either. */
+       if (skb_checksum_full(skb, 0, len, &csum)) {
+               if (!skb_shared(skb))
+                       skb->csum_valid = 0;
+               return (__force __sum16)1;
+       }
+
+       sum = csum_fold(csum);
        /* See comments in __skb_checksum_complete(). */
        if (likely(!sum)) {
                if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
@@ -3886,10 +3894,17 @@ EXPORT_SYMBOL(__skb_checksum_complete_head);
  */
 __sum16 __skb_checksum_complete(struct sk_buff *skb)
 {
-       __wsum csum;
+       __wsum csum = 0;
        __sum16 sum;

-       csum = skb_checksum(skb, 0, skb->len, 0);
+       /* If the data can't be checksummed it can't be valid either.  Leave
+        * skb->csum alone, it's not a full packet checksum.
+        */
+       if (skb_checksum_full(skb, 0, skb->len, &csum)) {
+               if (!skb_shared(skb))
+                       skb->csum_valid = 0;
+               return (__force __sum16)1;
+       }
I think maybe we need to be careful here which are all the skb flags
that we need to touch to indicate that the checksum failed.

My LLM is recommending:

        if (!skb_shared(skb)) {
            skb->ip_summed = CHECKSUM_NONE;
            skb->csum_complete_sw = 0;
            skb->csum_valid = 0;
        }

Seems ip_summed = *_NONE was caught elsewhere, but also maybe
csum_complete_sw=0 makes sense.

-- 
Thanks,
Mina
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help