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

[Josef Bacik <josef@xxxxxxxxxxxxxx>]

__skb_checksum_complete() and __skb_gro_checksum_complete() verify a
packet with skb_checksum(), which returns 0 when the payload is in
unreadable frags or the skb's lengths don't add up.  Both add that 0 to
the pseudo-header sum, so they accept the packet whenever the
pseudo-header sum folds to 0xffff.  The sender controls that sum through
the addresses and the length.

Use skb_checksum_full() in both, and in __skb_checksum_complete_head(),
and report an skb that can't be checksummed as having a bad checksum.

Also take the skb off CHECKSUM_COMPLETE on that path.  nf_ip_checksum()
stores the pseudo-header sum in skb->csum before it calls
__skb_checksum_complete(), and conntrack doesn't drop a packet with a
bad checksum.  If the skb stayed CHECKSUM_COMPLETE,
__skb_checksum_validate_complete() would add the same pseudo-header sum
to skb->csum and accept the packet under the same condition.

A test module builds an skb that claims 50 more bytes than it holds,
with a pseudo-header sum of 0xffff.  Both functions accept that skb
before this patch and reject it after.

Found by the Sashiko AI review of the BUG_ON() series.

Fixes: 20ddd0fd2fcd ("net: skbuff: don't BUG() on leftover length in 
skb_checksum() and friends")
Fixes: 65249feb6b3d ("net: add support for skbs with unreadable frags")
Link: 
https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@xxxxxxxxxxxxxx/
Link: 
https://lore.kernel.org/all/179273111951.3134.3452028799437573017@xxxxxxxxxx/
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 include/linux/skbuff.h |  4 ++--
 net/core/gro.c         | 11 +++++++++--
 net/core/skbuff.c      | 27 +++++++++++++++++++++++----
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 608c1491597e..5a5354a6b6ba 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4892,7 +4892,7 @@ static inline void skb_checksum_complete_unset(struct 
sk_buff *skb)
  *   0: checksum is validated or try to in skb_checksum_complete. In the latter
  *     case the ip_summed will not be CHECKSUM_UNNECESSARY and the pseudo
  *     checksum is stored in skb->csum for use in __skb_checksum_complete
- *   non-zero: value of invalid checksum
+ *   non-zero: value of invalid checksum, or 1 if the skb can't be checksummed
  *
  */
 static inline __sum16 __skb_checksum_validate_complete(struct sk_buff *skb,
@@ -4932,7 +4932,7 @@ static inline __wsum null_compute_pseudo(struct sk_buff 
*skb, int proto)
  *
  * Return values:
  *   0: checksum is validated or try to in skb_checksum_complete
- *   non-zero: value of invalid checksum
+ *   non-zero: value of invalid checksum, or 1 if the skb can't be checksummed
  */
 #define __skb_checksum_validate(skb, proto, complete,                  \
                                zero_okay, check, compute_pseudo)       \
diff --git a/net/core/gro.c b/net/core/gro.c
index 1b033855a20d..e2e847c95270 100644
--- a/net/core/gro.c
+++ b/net/core/gro.c
@@ -798,10 +798,17 @@ EXPORT_SYMBOL(napi_gro_frags);
  */
 __sum16 __skb_gro_checksum_complete(struct sk_buff *skb)
 {
-       __wsum wsum;
+       __wsum wsum = 0;
        __sum16 sum;
 
-       wsum = skb_checksum(skb, skb_gro_offset(skb), skb_gro_len(skb), 0);
+       /* If the data can't be checksummed it can't be valid either, and
+        * NAPI_GRO_CB(skb)->csum only holds the pseudo checksum.
+        */
+       if (skb_checksum_full(skb, skb_gro_offset(skb), skb_gro_len(skb),
+                             &wsum)) {
+               NAPI_GRO_CB(skb)->csum_valid = 0;
+               return (__force __sum16)1;
+       }
 
        /* NAPI_GRO_CB(skb)->csum holds pseudo checksum */
        sum = csum_fold(csum_add(NAPI_GRO_CB(skb)->csum, wsum));
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d51fdcb3e016..0082b12544cf 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3885,11 +3885,28 @@ u32 skb_crc32c(const struct sk_buff *skb, int offset, 
int len, u32 crc)
 EXPORT_SYMBOL(skb_crc32c);
 #endif /* CONFIG_NET_CRC32C */
 
+/* The data can't be checksummed, so it can't be valid either.  The caller has
+ * put the pseudo header's checksum in skb->csum, so don't leave the skb
+ * CHECKSUM_COMPLETE for a later validator to take that for the packet's.
+ */
+static __sum16 skb_checksum_complete_failed(struct sk_buff *skb)
+{
+       if (!skb_shared(skb)) {
+               skb_checksum_complete_unset(skb);
+               skb->csum_valid = 0;
+       }
+       return (__force __sum16)1;
+}
+
 __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 (skb_checksum_full(skb, 0, len, &csum))
+               return skb_checksum_complete_failed(skb);
+
+       sum = csum_fold(csum);
        /* See comments in __skb_checksum_complete(). */
        if (likely(!sum)) {
                if (unlikely(skb->ip_summed == CHECKSUM_COMPLETE) &&
@@ -3909,14 +3926,16 @@ EXPORT_SYMBOL(__skb_checksum_complete_head);
  *
  * It returns non-zero if the recomputed checksum is still invalid, otherwise
  * zero. The new checksum is stored back into skb->csum unless the skb is
- * shared.
+ * shared. If the skb can't be checksummed it returns non-zero, skb->csum is
+ * left alone, and the skb is no longer CHECKSUM_COMPLETE unless it's shared.
  */
 __sum16 __skb_checksum_complete(struct sk_buff *skb)
 {
-       __wsum csum;
+       __wsum csum = 0;
        __sum16 sum;
 
-       csum = skb_checksum(skb, 0, skb->len, 0);
+       if (skb_checksum_full(skb, 0, skb->len, &csum))
+               return skb_checksum_complete_failed(skb);
 
        sum = csum_fold(csum_add(skb->csum, csum));
        /* This check is inverted, because we already knew the hardware

-- 
2.55.0