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

[Mina Almasry <almasrymina@xxxxxxxxxx>]

On Mon, Oct 19, 2026 at 6:51 AM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> __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@xxxxxxxxxxxxxx/
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
> ---
>  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