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

[Stefan Bader]

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@fb.com/
> [1] https://lore.kernel.org/netdev/20180321233104.2142764-1-yhs@fb.com/
> 
> * 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 <stefan.bader at canonical.com>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210215/5070973c/attachment.sig>