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

[Fernando Fernandez Mancera <fmancera@xxxxxxx>]

On 10/21/26 3:46 PM, Josef Bacik 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>
Doesn't __skb_gro_checksum_complete() have the same problem? I think it 
could be fixed in this patch too.
> ---
>   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;
> +       }
>   
>   	sum = csum_fold(csum_add(skb->csum, csum));
>         /* This check is inverted, because we already knew the hardware