[B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Guilherme Piccoli
BugLink: https://bugs.launchpad.net/bugs/1915552


[Impact]
* It was reported upstream [0] that an eBPF NAT64 filter caused an oops due to
bad handling of GRO headers length on SKB segmentation path; the discussion is
rich in details, and eventually the reporter sent a fix patch for that [1], as
well as a test scenario in test_bpf kernel module that reproduces the issue.

[0] https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@.../
[1] https://lore.kernel.org/netdev/20180321233104.2142764-1-yhs@.../

* The fix patch landed on v4.17 and for some reason didn't reach the stable
kernels; by testing our Bionic v4.15 kernel I was able to reproduce the issue,
observing the following stack trace (details in the testing section below):

kernel BUG at net/core/skbuff.c:3703!
Modules linked in: test_bpf(E+) isofs nls_iso8859_1 dm_multipath ...
RIP: 0010:skb_segment+0xa34/0xce0
[...]
Call Trace:
 test_bpf_init.part.7+0x767/0x7d1 [test_bpf]
 test_bpf_init+0xfc/0x82f [test_bpf]
 do_one_initcall+0x52/0x19f
[...]

* Interesting to mention that this fix is not complete in the sense there was
another corner case reported after that [2], which was fixed by another
patch [3], this one released in kernel v5.3 and present in the stable tree
(hence backported to our Bionic 4.15 kernels).

[2] https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
[3] http://git.kernel.org/linus/3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")

* So we are hereby backporting both the original fix patch [4] as well
as the test_bpf patch (and a fix for it) [5][6] for Ubuntu Bionic v4.15-based
kernels.

[4] http://git.kernel.org/linus/13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb")
[5] http://git.kernel.org/linus/76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
[6] http://git.kernel.org/linus/99fe29d3a25f ("test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()")

[Test Case]
* One could use a NAT64 filter, but with the aforementioned patches [5][6]
hereby backported, one can also use the kernel infrastructure, by loading the
test_bpf module:

insmod /lib/modules/$(uname -r)/kernel/lib/test_bpf.ko

If patches [5] [6] are included and kernel doesn't contain the fix [4], an oops
will be observed.

[Where problems could occur]

* The backported patches are present upstream since v4.17, and no fixes were
released for them (other than [6], included here), so from the testing
point-of-view, these patches are being exercised for a while with no issues.

* That said, if a problem would be triggered by these patches, hypothetically
it would affect SKB segmentation, the net/core code - a bad check could case an
oops in this code or they could present a pretty small overhead due to more
checks in the hot path.


Dan Carpenter (1):
  test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()

Yonghong Song (2):
  net: permit skb_segment on head_frag frag_list skb
  net: bpf: add a test for skb_segment in test_bpf module

 lib/test_bpf.c    | 93 ++++++++++++++++++++++++++++++++++++++++++++++-
 net/core/skbuff.c | 26 +++++++++++--
 2 files changed, 113 insertions(+), 6 deletions(-)

--
2.29.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[B][PATCH 1/3] net: permit skb_segment on head_frag frag_list skb

Guilherme Piccoli
From: Yonghong Song <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1915552

One of our in-house projects, bpf-based NAT, hits a kernel BUG_ON at
function skb_segment(), line 3667. The bpf program attaches to
clsact ingress, calls bpf_skb_change_proto to change protocol
from ipv4 to ipv6 or from ipv6 to ipv4, and then calls bpf_redirect
to send the changed packet out.

3472 struct sk_buff *skb_segment(struct sk_buff *head_skb,
3473                             netdev_features_t features)
3474 {
3475         struct sk_buff *segs = NULL;
3476         struct sk_buff *tail = NULL;
...
3665                 while (pos < offset + len) {
3666                         if (i >= nfrags) {
3667                                 BUG_ON(skb_headlen(list_skb));
3668
3669                                 i = 0;
3670                                 nfrags = skb_shinfo(list_skb)->nr_frags;
3671                                 frag = skb_shinfo(list_skb)->frags;
3672                                 frag_skb = list_skb;
...

call stack:
...
 #1 [ffff883ffef03558] __crash_kexec at ffffffff8110c525
 #2 [ffff883ffef03620] crash_kexec at ffffffff8110d5cc
 #3 [ffff883ffef03640] oops_end at ffffffff8101d7e7
 #4 [ffff883ffef03668] die at ffffffff8101deb2
 #5 [ffff883ffef03698] do_trap at ffffffff8101a700
 #6 [ffff883ffef036e8] do_error_trap at ffffffff8101abfe
 #7 [ffff883ffef037a0] do_invalid_op at ffffffff8101acd0
 #8 [ffff883ffef037b0] invalid_op at ffffffff81a00bab
    [exception RIP: skb_segment+3044]
    RIP: ffffffff817e4dd4  RSP: ffff883ffef03860  RFLAGS: 00010216
    RAX: 0000000000002bf6  RBX: ffff883feb7aaa00  RCX: 0000000000000011
    RDX: ffff883fb87910c0  RSI: 0000000000000011  RDI: ffff883feb7ab500
    RBP: ffff883ffef03928   R8: 0000000000002ce2   R9: 00000000000027da
    R10: 000001ea00000000  R11: 0000000000002d82  R12: ffff883f90a1ee80
    R13: ffff883fb8791120  R14: ffff883feb7abc00  R15: 0000000000002ce2
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #9 [ffff883ffef03930] tcp_gso_segment at ffffffff818713e7

Signed-off-by: David S. Miller <[hidden email]>
(backported from commit 13acc94eff122b260a387d68668bf9d670738e6a)
[gpiccoli: context adjustment due to the lack of upstream commit
bf5c25d60861 ("skbuff: in skb_segment, call zerocopy functions once per nskb")
in our 4.15 kernels.]
[gpiccoli: The commit message was for some reason cut during the merge
upstream; to check the full commit, with author's signoff, see:
https://lore.kernel.org/netdev/20180321233104.2142764-2-yhs@... .]
Signed-off-by: Guilherme G. Piccoli <[hidden email]>
---
 net/core/skbuff.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8400b07b4fac..d6d93cc41ad5 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3492,6 +3492,19 @@ void *skb_pull_rcsum(struct sk_buff *skb, unsigned int len)
 }
 EXPORT_SYMBOL_GPL(skb_pull_rcsum);
 
+static inline skb_frag_t skb_head_frag_to_page_desc(struct sk_buff *frag_skb)
+{
+ skb_frag_t head_frag;
+ struct page *page;
+
+ page = virt_to_head_page(frag_skb->head);
+ head_frag.page.p = page;
+ head_frag.page_offset = frag_skb->data -
+ (unsigned char *)page_address(page);
+ head_frag.size = skb_headlen(frag_skb);
+ return head_frag;
+}
+
 /**
  * skb_segment - Perform protocol segmentation on skb.
  * @head_skb: buffer to segment
@@ -3711,14 +3724,19 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
 
  while (pos < offset + len) {
  if (i >= nfrags) {
- BUG_ON(skb_headlen(list_skb));
-
  i = 0;
  nfrags = skb_shinfo(list_skb)->nr_frags;
  frag = skb_shinfo(list_skb)->frags;
  frag_skb = list_skb;
+ if (!skb_headlen(list_skb)) {
+ BUG_ON(!nfrags);
+ } else {
+ BUG_ON(!list_skb->head_frag);
 
- BUG_ON(!nfrags);
+ /* to make room for head_frag. */
+ i--;
+ frag--;
+ }
 
  list_skb = list_skb->next;
  }
@@ -3737,7 +3755,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
  if (skb_zerocopy_clone(nskb, frag_skb, GFP_ATOMIC))
  goto err;
 
- *nskb_frag = *frag;
+ *nskb_frag = (i < 0) ? skb_head_frag_to_page_desc(frag_skb) : *frag;
  __skb_frag_ref(nskb_frag);
  size = skb_frag_size(nskb_frag);
 
--
2.29.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[B][PATCH 2/3] net: bpf: add a test for skb_segment in test_bpf module

Guilherme Piccoli
In reply to this post by Guilherme Piccoli
From: Yonghong Song <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1915552

Without the previous commit,
"modprobe test_bpf" will have the following errors:
...
[   98.149165] ------------[ cut here ]------------
[   98.159362] kernel BUG at net/core/skbuff.c:3667!
[   98.169756] invalid opcode: 0000 [#1] SMP PTI
[   98.179370] Modules linked in:
[   98.179371]  test_bpf(+)
...
which triggers the bug the previous commit intends to fix.

The skbs are constructed to mimic what mlx5 may generate.
The packet size/header may not mimic real cases in production. But
the processing flow is similar.

Signed-off-by: Yonghong Song <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 76db8087c4c991dcd17f5ea8ac0eafd0696ab450)
Signed-off-by: Guilherme G. Piccoli <[hidden email]>
---
 lib/test_bpf.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 2 deletions(-)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 1fe4d4b33217..3f0821cb0ab4 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6482,6 +6482,93 @@ static bool exclude_test(int test_id)
  return test_id < test_range[0] || test_id > test_range[1];
 }
 
+static __init struct sk_buff *build_test_skb(void)
+{
+ u32 headroom = NET_SKB_PAD + NET_IP_ALIGN + ETH_HLEN;
+ struct sk_buff *skb[2];
+ struct page *page[2];
+ int i, data_size = 8;
+
+ for (i = 0; i < 2; i++) {
+ page[i] = alloc_page(GFP_KERNEL);
+ if (!page[i]) {
+ if (i == 0)
+ goto err_page0;
+ else
+ goto err_page1;
+ }
+
+ /* this will set skb[i]->head_frag */
+ skb[i] = dev_alloc_skb(headroom + data_size);
+ if (!skb[i]) {
+ if (i == 0)
+ goto err_skb0;
+ else
+ goto err_skb1;
+ }
+
+ skb_reserve(skb[i], headroom);
+ skb_put(skb[i], data_size);
+ skb[i]->protocol = htons(ETH_P_IP);
+ skb_reset_network_header(skb[i]);
+ skb_set_mac_header(skb[i], -ETH_HLEN);
+
+ skb_add_rx_frag(skb[i], 0, page[i], 0, 64, 64);
+ // skb_headlen(skb[i]): 8, skb[i]->head_frag = 1
+ }
+
+ /* setup shinfo */
+ skb_shinfo(skb[0])->gso_size = 1448;
+ skb_shinfo(skb[0])->gso_type = SKB_GSO_TCPV4;
+ skb_shinfo(skb[0])->gso_type |= SKB_GSO_DODGY;
+ skb_shinfo(skb[0])->gso_segs = 0;
+ skb_shinfo(skb[0])->frag_list = skb[1];
+
+ /* adjust skb[0]'s len */
+ skb[0]->len += skb[1]->len;
+ skb[0]->data_len += skb[1]->data_len;
+ skb[0]->truesize += skb[1]->truesize;
+
+ return skb[0];
+
+err_skb1:
+ __free_page(page[1]);
+err_page1:
+ kfree_skb(skb[0]);
+err_skb0:
+ __free_page(page[0]);
+err_page0:
+ return NULL;
+}
+
+static __init int test_skb_segment(void)
+{
+ netdev_features_t features;
+ struct sk_buff *skb, *segs;
+ int ret = -1;
+
+ features = NETIF_F_SG | NETIF_F_GSO_PARTIAL | NETIF_F_IP_CSUM |
+   NETIF_F_IPV6_CSUM;
+ features |= NETIF_F_RXCSUM;
+ skb = build_test_skb();
+ if (!skb) {
+ pr_info("%s: failed to build_test_skb", __func__);
+ goto done;
+ }
+
+ segs = skb_segment(skb, features);
+ if (segs) {
+ kfree_skb_list(segs);
+ ret = 0;
+ pr_info("%s: success in skb_segment!", __func__);
+ } else {
+ pr_info("%s: failed in skb_segment!", __func__);
+ }
+ kfree_skb(skb);
+done:
+ return ret;
+}
+
 static __init int test_bpf(void)
 {
  int i, err_cnt = 0, pass_cnt = 0;
@@ -6539,9 +6626,11 @@ static int __init test_bpf_init(void)
  return ret;
 
  ret = test_bpf();
-
  destroy_bpf_tests();
- return ret;
+ if (ret)
+ return ret;
+
+ return test_skb_segment();
 }
 
 static void __exit test_bpf_exit(void)
--
2.29.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[B][PATCH 3/3] test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()

Guilherme Piccoli
In reply to this post by Guilherme Piccoli
From: Dan Carpenter <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1915552

The skb_segment() function returns error pointers on error.  It never
returns NULL.

Fixes: 76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
Signed-off-by: Dan Carpenter <[hidden email]>
Acked-by: Daniel Borkmann <[hidden email]>
Reviewed-by: Yonghong Song <[hidden email]>
Signed-off-by: David S. Miller <[hidden email]>
(cherry picked from commit 99fe29d3a25f813146816b322367b4c6a0fdec84)
Signed-off-by: Guilherme G. Piccoli <[hidden email]>
---
 lib/test_bpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 3f0821cb0ab4..9f5530b8a2f8 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -6557,7 +6557,7 @@ static __init int test_skb_segment(void)
  }
 
  segs = skb_segment(skb, features);
- if (segs) {
+ if (!IS_ERR(segs)) {
  kfree_skb_list(segs);
  ret = 0;
  pr_info("%s: success in skb_segment!", __func__);
--
2.29.0


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Thadeu Lima de Souza Cascardo-3
In reply to this post by Guilherme Piccoli
On Fri, Feb 12, 2021 at 05:41:56PM -0300, Guilherme G. Piccoli wrote:
> BugLink: https://bugs.launchpad.net/bugs/1915552
>

Nice writeup. Good testing for the bug.

Acked-by: Thadeu Lima de Souza Cascardo <[hidden email]>

>
> [Impact]
> * It was reported upstream [0] that an eBPF NAT64 filter caused an oops due to
> bad handling of GRO headers length on SKB segmentation path; the discussion is
> rich in details, and eventually the reporter sent a fix patch for that [1], as
> well as a test scenario in test_bpf kernel module that reproduces the issue.
>
> [0] https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@.../
> [1] https://lore.kernel.org/netdev/20180321233104.2142764-1-yhs@.../
>
> * The fix patch landed on v4.17 and for some reason didn't reach the stable
> kernels; by testing our Bionic v4.15 kernel I was able to reproduce the issue,
> observing the following stack trace (details in the testing section below):
>
> kernel BUG at net/core/skbuff.c:3703!
> Modules linked in: test_bpf(E+) isofs nls_iso8859_1 dm_multipath ...
> RIP: 0010:skb_segment+0xa34/0xce0
> [...]
> Call Trace:
>  test_bpf_init.part.7+0x767/0x7d1 [test_bpf]
>  test_bpf_init+0xfc/0x82f [test_bpf]
>  do_one_initcall+0x52/0x19f
> [...]
>
> * Interesting to mention that this fix is not complete in the sense there was
> another corner case reported after that [2], which was fixed by another
> patch [3], this one released in kernel v5.3 and present in the stable tree
> (hence backported to our Bionic 4.15 kernels).
>
> [2] https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
> [3] http://git.kernel.org/linus/3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
>
> * So we are hereby backporting both the original fix patch [4] as well
> as the test_bpf patch (and a fix for it) [5][6] for Ubuntu Bionic v4.15-based
> kernels.
>
> [4] http://git.kernel.org/linus/13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb")
> [5] http://git.kernel.org/linus/76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> [6] http://git.kernel.org/linus/99fe29d3a25f ("test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()")
>
> [Test Case]
> * One could use a NAT64 filter, but with the aforementioned patches [5][6]
> hereby backported, one can also use the kernel infrastructure, by loading the
> test_bpf module:
>
> insmod /lib/modules/$(uname -r)/kernel/lib/test_bpf.ko
>
> If patches [5] [6] are included and kernel doesn't contain the fix [4], an oops
> will be observed.
>
> [Where problems could occur]
>
> * The backported patches are present upstream since v4.17, and no fixes were
> released for them (other than [6], included here), so from the testing
> point-of-view, these patches are being exercised for a while with no issues.
>
> * That said, if a problem would be triggered by these patches, hypothetically
> it would affect SKB segmentation, the net/core code - a bad check could case an
> oops in this code or they could present a pretty small overhead due to more
> checks in the hot path.
>
>
> Dan Carpenter (1):
>   test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
>
> Yonghong Song (2):
>   net: permit skb_segment on head_frag frag_list skb
>   net: bpf: add a test for skb_segment in test_bpf module
>
>  lib/test_bpf.c    | 93 ++++++++++++++++++++++++++++++++++++++++++++++-
>  net/core/skbuff.c | 26 +++++++++++--
>  2 files changed, 113 insertions(+), 6 deletions(-)
>
> --
> 2.29.0
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Stefan Bader-2
In reply to this post by Guilherme Piccoli
On 12.02.21 21:41, Guilherme G. Piccoli wrote:

> BugLink: https://bugs.launchpad.net/bugs/1915552
>
>
> [Impact]
> * It was reported upstream [0] that an eBPF NAT64 filter caused an oops due to
> bad handling of GRO headers length on SKB segmentation path; the discussion is
> rich in details, and eventually the reporter sent a fix patch for that [1], as
> well as a test scenario in test_bpf kernel module that reproduces the issue.
>
> [0] https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@.../
> [1] https://lore.kernel.org/netdev/20180321233104.2142764-1-yhs@.../
>
> * The fix patch landed on v4.17 and for some reason didn't reach the stable
> kernels; by testing our Bionic v4.15 kernel I was able to reproduce the issue,
> observing the following stack trace (details in the testing section below):
>
> kernel BUG at net/core/skbuff.c:3703!
> Modules linked in: test_bpf(E+) isofs nls_iso8859_1 dm_multipath ...
> RIP: 0010:skb_segment+0xa34/0xce0
> [...]
> Call Trace:
>  test_bpf_init.part.7+0x767/0x7d1 [test_bpf]
>  test_bpf_init+0xfc/0x82f [test_bpf]
>  do_one_initcall+0x52/0x19f
> [...]
>
> * Interesting to mention that this fix is not complete in the sense there was
> another corner case reported after that [2], which was fixed by another
> patch [3], this one released in kernel v5.3 and present in the stable tree
> (hence backported to our Bionic 4.15 kernels).
>
> [2] https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
> [3] http://git.kernel.org/linus/3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
>
> * So we are hereby backporting both the original fix patch [4] as well
> as the test_bpf patch (and a fix for it) [5][6] for Ubuntu Bionic v4.15-based
> kernels.
>
> [4] http://git.kernel.org/linus/13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb")
> [5] http://git.kernel.org/linus/76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> [6] http://git.kernel.org/linus/99fe29d3a25f ("test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()")
>
> [Test Case]
> * One could use a NAT64 filter, but with the aforementioned patches [5][6]
> hereby backported, one can also use the kernel infrastructure, by loading the
> test_bpf module:
>
> insmod /lib/modules/$(uname -r)/kernel/lib/test_bpf.ko
>
> If patches [5] [6] are included and kernel doesn't contain the fix [4], an oops
> will be observed.
>
> [Where problems could occur]
>
> * The backported patches are present upstream since v4.17, and no fixes were
> released for them (other than [6], included here), so from the testing
> point-of-view, these patches are being exercised for a while with no issues.
>
> * That said, if a problem would be triggered by these patches, hypothetically
> it would affect SKB segmentation, the net/core code - a bad check could case an
> oops in this code or they could present a pretty small overhead due to more
> checks in the hot path.
>
>
> Dan Carpenter (1):
>   test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
>
> Yonghong Song (2):
>   net: permit skb_segment on head_frag frag_list skb
>   net: bpf: add a test for skb_segment in test_bpf module
>
>  lib/test_bpf.c    | 93 ++++++++++++++++++++++++++++++++++++++++++++++-
>  net/core/skbuff.c | 26 +++++++++++--
>  2 files changed, 113 insertions(+), 6 deletions(-)
>
For users only the first patch is relevant and as far as one can tell with net
code there is still a case in the flow that is as before with an additional case
added. And the next 2 patches add a way to test this. So

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Stefan Bader-2
In reply to this post by Guilherme Piccoli
On 12.02.21 21:41, Guilherme G. Piccoli wrote:

> BugLink: https://bugs.launchpad.net/bugs/1915552
>
>
> [Impact]
> * It was reported upstream [0] that an eBPF NAT64 filter caused an oops due to
> bad handling of GRO headers length on SKB segmentation path; the discussion is
> rich in details, and eventually the reporter sent a fix patch for that [1], as
> well as a test scenario in test_bpf kernel module that reproduces the issue.
>
> [0] https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@.../
> [1] https://lore.kernel.org/netdev/20180321233104.2142764-1-yhs@.../
>
> * The fix patch landed on v4.17 and for some reason didn't reach the stable
> kernels; by testing our Bionic v4.15 kernel I was able to reproduce the issue,
> observing the following stack trace (details in the testing section below):
>
> kernel BUG at net/core/skbuff.c:3703!
> Modules linked in: test_bpf(E+) isofs nls_iso8859_1 dm_multipath ...
> RIP: 0010:skb_segment+0xa34/0xce0
> [...]
> Call Trace:
>  test_bpf_init.part.7+0x767/0x7d1 [test_bpf]
>  test_bpf_init+0xfc/0x82f [test_bpf]
>  do_one_initcall+0x52/0x19f
> [...]
>
> * Interesting to mention that this fix is not complete in the sense there was
> another corner case reported after that [2], which was fixed by another
> patch [3], this one released in kernel v5.3 and present in the stable tree
> (hence backported to our Bionic 4.15 kernels).
>
> [2] https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
> [3] http://git.kernel.org/linus/3dcbdb134f32 ("net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list")
>
> * So we are hereby backporting both the original fix patch [4] as well
> as the test_bpf patch (and a fix for it) [5][6] for Ubuntu Bionic v4.15-based
> kernels.
>
> [4] http://git.kernel.org/linus/13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb")
> [5] http://git.kernel.org/linus/76db8087c4c9 ("net: bpf: add a test for skb_segment in test_bpf module")
> [6] http://git.kernel.org/linus/99fe29d3a25f ("test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()")
>
> [Test Case]
> * One could use a NAT64 filter, but with the aforementioned patches [5][6]
> hereby backported, one can also use the kernel infrastructure, by loading the
> test_bpf module:
>
> insmod /lib/modules/$(uname -r)/kernel/lib/test_bpf.ko
>
> If patches [5] [6] are included and kernel doesn't contain the fix [4], an oops
> will be observed.
>
> [Where problems could occur]
>
> * The backported patches are present upstream since v4.17, and no fixes were
> released for them (other than [6], included here), so from the testing
> point-of-view, these patches are being exercised for a while with no issues.
>
> * That said, if a problem would be triggered by these patches, hypothetically
> it would affect SKB segmentation, the net/core code - a bad check could case an
> oops in this code or they could present a pretty small overhead due to more
> checks in the hot path.
>
>
> Dan Carpenter (1):
>   test_bpf: Fix NULL vs IS_ERR() check in test_skb_segment()
>
> Yonghong Song (2):
>   net: permit skb_segment on head_frag frag_list skb
>   net: bpf: add a test for skb_segment in test_bpf module
>
>  lib/test_bpf.c    | 93 ++++++++++++++++++++++++++++++++++++++++++++++-
>  net/core/skbuff.c | 26 +++++++++++--
>  2 files changed, 113 insertions(+), 6 deletions(-)
>
One thing I realized after sending the ack. Somehow the bug report and
submission were not matching on the source. I assumed from the descriptions that
the Bionic primary kernel was correct (and not linux-azure). If that is wrong,
you have to shout rather sooner than later.

-Stefan

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Guilherme Piccoli
Thanks for noticing that Stefan! I guess I've used an opened
linux-azure LP to produce the new LP and got this confused hehe
Indeed, the correct is Linux/generic, in order it get delivered
automatically to all 4.15-based flavors. Seems it's fixed now, right?

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [B][PATCH 0/3] Fix oops in skb_segment for Bionic series

Stefan Bader-2
On 15.02.21 13:07, Guilherme Piccoli wrote:
> Thanks for noticing that Stefan! I guess I've used an opened
> linux-azure LP to produce the new LP and got this confused hehe
> Indeed, the correct is Linux/generic, in order it get delivered
> automatically to all 4.15-based flavors. Seems it's fixed now, right?
>

Yes, all good. Applied to bionic:linux/master-next. Thanks.

-Stefan

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team