diff options
| author | Jakub Kicinski <kuba@kernel.org> | 2022-12-19 16:47:00 -0800 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2023-01-12 12:01:57 +0100 |
| commit | 321635c70352d635832a2cb067520cbc0852bc7b (patch) | |
| tree | c8724f56538369137ca99f59583a9a4c64115a99 /net/core | |
| parent | 35dab092c295b76d94bd5b47ff60a11190e532ce (diff) | |
| download | linux-stable-321635c70352d635832a2cb067520cbc0852bc7b.tar.gz linux-stable-321635c70352d635832a2cb067520cbc0852bc7b.tar.bz2 linux-stable-321635c70352d635832a2cb067520cbc0852bc7b.zip | |
bpf: pull before calling skb_postpull_rcsum()
[ Upstream commit 54c3f1a81421f85e60ae2eaae7be3727a09916ee ]
Anand hit a BUG() when pulling off headers on egress to a SW tunnel.
We get to skb_checksum_help() with an invalid checksum offset
(commit d7ea0d9df2a6 ("net: remove two BUG() from skb_checksum_help()")
converted those BUGs to WARN_ONs()).
He points out oddness in how skb_postpull_rcsum() gets used.
Indeed looks like we should pull before "postpull", otherwise
the CHECKSUM_PARTIAL fixup from skb_postpull_rcsum() will not
be able to do its job:
if (skb->ip_summed == CHECKSUM_PARTIAL &&
skb_checksum_start_offset(skb) < 0)
skb->ip_summed = CHECKSUM_NONE;
Reported-by: Anand Parthasarathy <anpartha@meta.com>
Fixes: 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Stanislav Fomichev <sdf@google.com>
Link: https://lore.kernel.org/r/20221220004701.402165-1-kuba@kernel.org
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Diffstat (limited to 'net/core')
| -rw-r--r-- | net/core/filter.c | 7 |
1 files changed, 5 insertions, 2 deletions
diff --git a/net/core/filter.c b/net/core/filter.c index a368edd9057c..0c2666e041d3 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3184,15 +3184,18 @@ static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len) static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len) { + void *old_data; + /* skb_ensure_writable() is not needed here, as we're * already working on an uncloned skb. */ if (unlikely(!pskb_may_pull(skb, off + len))) return -ENOMEM; - skb_postpull_rcsum(skb, skb->data + off, len); - memmove(skb->data + len, skb->data, off); + old_data = skb->data; __skb_pull(skb, len); + skb_postpull_rcsum(skb, old_data + off, len); + memmove(skb->data, old_data, off); return 0; } |