[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20211203185238.2011081-1-eric.dumazet@gmail.com>
Date: Fri, 3 Dec 2021 10:52:38 -0800
From: Eric Dumazet <eric.dumazet@...il.com>
To: "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
Cc: netdev <netdev@...r.kernel.org>,
Eric Dumazet <edumazet@...gle.com>,
Eric Dumazet <eric.dumazet@...il.com>,
Vladimir Oltean <vladimir.oltean@....com>,
David Laight <David.Laight@...LAB.COM>,
David Lebrun <dlebrun@...gle.com>
Subject: [PATCH net-next] net: fix recent csum changes
From: Eric Dumazet <edumazet@...gle.com>
Vladimir reported csum issues after my recent change in skb_postpull_rcsum()
Issue here is the following:
initial skb->csum is the csum of
[part to be pulled][rest of packet]
Old code:
skb->csum = csum_sub(skb->csum, csum_partial(pull, pull_length, 0));
New code:
skb->csum = ~csum_partial(pull, pull_length, ~skb->csum);
This is broken if the csum of [pulled part]
happens to be equal to skb->csum, because end
result of skb->csum is 0 in new code, instead
of being 0xffffffff
David Laight suggested to use
skb->csum = -csum_partial(pull, pull_length, -skb->csum);
I based my patches on existing code present in include/net/seg6.h,
update_csum_diff4() and update_csum_diff16() which might need
a similar fix.
I guess that my tests, mostly pulling 40 bytes of IPv6 header
were not providing enough entropy to hit this bug.
Fixes: 29c3002644bd ("net: optimize skb_postpull_rcsum()")
Fixes: 0bd28476f636 ("gro: optimize skb_gro_postpull_rcsum()")
Signed-off-by: Eric Dumazet <edumazet@...gle.com>
Reported-by: Vladimir Oltean <vladimir.oltean@....com>
Suggested-by: David Laight <David.Laight@...LAB.COM>
Cc: David Lebrun <dlebrun@...gle.com>
---
include/linux/skbuff.h | 2 +-
include/net/gro.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index eae4bd3237a41cc1b60b44c91fbfe21dfdd8f117..2bbcdaf99ed3df739ddfd2de4be747262226d4b9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3486,7 +3486,7 @@ static inline void skb_postpull_rcsum(struct sk_buff *skb,
const void *start, unsigned int len)
{
if (skb->ip_summed == CHECKSUM_COMPLETE)
- skb->csum = ~csum_partial(start, len, ~skb->csum);
+ skb->csum = -csum_partial(start, len, -skb->csum);
else if (skb->ip_summed == CHECKSUM_PARTIAL &&
skb_checksum_start_offset(skb) < 0)
skb->ip_summed = CHECKSUM_NONE;
diff --git a/include/net/gro.h b/include/net/gro.h
index b1139fca7c435ca0c353c4ed17628dd7f3df4401..4529c4c6f3ca4ac23da569b0bc0e00e3c9dcd765 100644
--- a/include/net/gro.h
+++ b/include/net/gro.h
@@ -173,8 +173,8 @@ static inline void skb_gro_postpull_rcsum(struct sk_buff *skb,
const void *start, unsigned int len)
{
if (NAPI_GRO_CB(skb)->csum_valid)
- NAPI_GRO_CB(skb)->csum = ~csum_partial(start, len,
- ~NAPI_GRO_CB(skb)->csum);
+ NAPI_GRO_CB(skb)->csum = -csum_partial(start, len,
+ -NAPI_GRO_CB(skb)->csum);
}
/* GRO checksum functions. These are logical equivalents of the normal
--
2.34.1.400.ga245620fadb-goog
Powered by blists - more mailing lists