All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fred Li <dracodingfly@gmail.com>
To: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, aleksander.lobakin@intel.com,
	sashal@kernel.org, linux@weissschuh.net, hawk@kernel.org,
	nbd@nbd.name, mkhalfella@purestorage.com, ast@kernel.org,
	daniel@iogearbox.net, andrii@kernel.org, martin.lau@linux.dev,
	song@kernel.org, yonghong.song@linux.dev,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org, Fred Li <dracodingfly@gmail.com>
Subject: [PATCH v2 1/2] net: Fix skb_segment when splitting gso_size mangled skb having linear-headed frag_list whose head_frag=true
Date: Wed, 26 Jun 2024 14:55:54 +0800	[thread overview]
Message-ID: <20240626065555.35460-2-dracodingfly@gmail.com> (raw)
In-Reply-To: <20240626065555.35460-1-dracodingfly@gmail.com>

When using calico bpf based NAT, hits a kernel BUG_ON at function
skb_segment(), line 4560. Performing NAT translation when accessing
a Service IP across subnets, the calico will encap vxlan and calls the
bpf_skb_adjust_room to decrease the gso_size, and then call bpf_redirect
send packets out.

4438 struct sk_buff *skb_segment(struct sk_buff *head_skb,
4439                             netdev_features_t features)
4440 {
4441     struct sk_buff *segs = NULL;
4442     struct sk_buff *tail = NULL;
...
4558         if (hsize <= 0 && i >= nfrags && skb_headlen(list_skb) &&
4559             (skb_headlen(list_skb) == len || sg)) {
4560                 BUG_ON(skb_headlen(list_skb) > len);
4561
4562                 nskb = skb_clone(list_skb, GFP_ATOMIC);
4563                 if (unlikely(!nskb))
4564                     goto err;

call stack:
...
   [exception RIP: skb_segment+3016]
    RIP: ffffffffb97df2a8  RSP: ffffa3f2cce08728  RFLAGS: 00010293
    RAX: 000000000000007d  RBX: 00000000fffff7b3  RCX: 0000000000000011
    RDX: 0000000000000000  RSI: ffff895ea32c76c0  RDI: 00000000000008c1
    RBP: ffffa3f2cce087f8   R8: 000000000000088f   R9: 0000000000000011
    R10: 000000000000090c  R11: ffff895e47e68000  R12: ffff895eb2022f00
    R13: 000000000000004b  R14: ffff895ecdaf2000  R15: ffff895eb2023f00
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #9 [ffffa3f2cce08720] skb_segment at ffffffffb97ded63
...

The skb has the following properties:
    doffset = 66
    list_skb = skb_shinfo(skb)->frag_list
    list_skb->head_frag = true
    skb->len = 2441 && skb->data_len = 2250
    skb_shinfo(skb)->nr_frags = 17
    skb_shinfo(skb)->gso_size = 75
    skb_shinfo(skb)->frags[0...16].bv_len = 125
    list_skb->len = 125
    list_skb->data_len = 0

When slicing the frag_list skb, there three cases:
1. Only *non* head_frag
    sg will be false, only when skb_headlen(list_skb)==len is satisfied,
    it will enter the branch at line 4560, and there will be no crash.
2. Mixed head_frag
    sg will be false, Only when skb_headlen(list_skb)==len is satisfied,
    it will enter the branch at line 4560, and there will be no crash.
3. Only frag_list with head_frag=true
    sg is true, three cases below:
    (1) skb_headlen(list_skb)==len is satisfied, it will enter the branch
       at line 4560, and there will be no crash.
    (2) skb_headlen(list_skb)<len is satisfied, it will enter the branch
       at line 4560, and there will be no crash.
    (3) skb_headlen(list_skb)>len is satisfied, it will be crash.

Applying this patch, three cases will be:
1. Only *non* head_frag
    sg will be false. No difference with before.
2. Mixed head_frag
    sg will be false. No difference with before.
3. Only frag_list with head_frag=true
    sg is true, there also three cases:
    (1) skb_headlen(list_skb)==len is satisfied, no difference with before.
    (2) skb_headlen(list_skb)<len is satisfied, will be revert to copying
        in this case.
    (3) skb_headlen(list_skb)>len is satisfied, will be revert to copying
        in this case.

Since commit 13acc94eff122("net: permit skb_segment on head_frag frag_list
skb"), it is allowed to segment the head_frag frag_list skb.

Signed-off-by: Fred Li <dracodingfly@gmail.com>
---
 net/core/skbuff.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f0a9ef1aeaa2..b1dab1b071fc 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -4556,7 +4556,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 		hsize = skb_headlen(head_skb) - offset;
 
 		if (hsize <= 0 && i >= nfrags && skb_headlen(list_skb) &&
-		    (skb_headlen(list_skb) == len || sg)) {
+		    (skb_headlen(list_skb) == len)) {
 			BUG_ON(skb_headlen(list_skb) > len);
 
 			nskb = skb_clone(list_skb, GFP_ATOMIC);
-- 
2.33.0


  reply	other threads:[~2024-06-26  6:56 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-26  6:55 [PATCH v2 2/2] test_bpf: Introduce an skb_segment test for frag_list whose head_frag=true and gso_size was mangled Fred Li
2024-06-26  6:55 ` Fred Li [this message]
2024-07-02  9:23   ` [PATCH v2 1/2] net: Fix skb_segment when splitting gso_size mangled skb having linear-headed frag_list whose head_frag=true Paolo Abeni
2024-07-03 12:21     ` Fred Li
2024-07-06 14:26       ` Willem de Bruijn
2024-07-08 14:31         ` [PATCH] net: linearizing skb when downgrade gso_size Fred Li
2024-07-09 15:53           ` Willem de Bruijn
2024-07-09 20:16             ` Herbert Xu
2024-07-09 21:29               ` Willem de Bruijn
2024-07-10 23:06                 ` Herbert Xu
2024-07-12  8:17             ` Fred Li
2024-07-17  5:35         ` [PATCH v3] " Fred Li
2024-07-17 16:32           ` Willem de Bruijn
2024-07-18  7:42             ` Fred Li
2024-07-19  2:46         ` [PATCH v4] bpf: Fixed a segment issue " Fred Li
2024-07-19 18:22           ` Willem de Bruijn
2024-07-23  1:12           ` bot+bpf-ci
2024-07-22  3:08         ` [PATCH bpf v5] bpf: Fixed " Fred Li
2024-07-22  3:34           ` bot+bpf-ci
2024-07-22 16:29           ` Willem de Bruijn
2024-07-24 13:37             ` Fred Li
2024-07-25 10:09               ` Daniel Borkmann
2024-06-26  6:55 ` [PATCH v2 0/2] net: Fix BUG_ON for segment frag_list with mangled gso_size and head_frag is true Fred Li

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240626065555.35460-2-dracodingfly@gmail.com \
    --to=dracodingfly@gmail.com \
    --cc=aleksander.lobakin@intel.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=haoluo@google.com \
    --cc=hawk@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=martin.lau@linux.dev \
    --cc=mkhalfella@purestorage.com \
    --cc=nbd@nbd.name \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sashal@kernel.org \
    --cc=sdf@google.com \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.