lwt xmit hook does not expect positive return values in function
ip_finish_output2 and ip6_finish_output. However, BPF programs can
directly return positive statuses such like NET_XMIT_DROP, NET_RX_DROP,
and etc to the caller. Such return values would make the kernel continue
processing already freed skbs and eventually panic.

This set fixes the return values from BPF ops to unexpected continue
processing, checks strictly on the correct continue condition for
future proof. In addition, add missing selftests for BPF redirect
and reroute cases for BPF-CI.

v5: https://lore.kernel.org/bpf/cover.1692153515.git.yan@cloudflare.com/ 
v4: https://lore.kernel.org/bpf/ZMD1sFTW8SFiex+x@debian.debian/T/ 
v3: https://lore.kernel.org/bpf/cover.1690255889.git.yan@cloudflare.com/ 
v2: https://lore.kernel.org/netdev/ZLdY6JkWRccunvu0@debian.debian/ 
v1: https://lore.kernel.org/bpf/ZLbYdpWC8zt9EJtq@debian.debian/ 

changes since v5:
 * fix BPF-CI failures due to missing config and busybox ping issue

changes since v4:
 * fixed same error on BPF_REROUTE path
 * re-implemented selftests under BPF-CI requirement

changes since v3:
 * minor change in commit message and changelogs
 * tested by Jakub Sitnicki

changes since v2:
 * subject name changed
 * also covered redirect to ingress case
 * added selftests

changes since v1:
 * minor code style changes

Yan Zhai (4):
  lwt: fix return values of BPF xmit ops
  lwt: check LWTUNNEL_XMIT_CONTINUE strictly
  selftests/bpf: add lwt_xmit tests for BPF_REDIRECT
  selftests/bpf: add lwt_xmit tests for BPF_REROUTE

 include/net/lwtunnel.h                        |   5 +-
 net/core/lwt_bpf.c                            |   7 +-
 net/ipv4/ip_output.c                          |   2 +-
 net/ipv6/ip6_output.c                         |   2 +-
 tools/testing/selftests/bpf/config            |   2 +
 .../selftests/bpf/prog_tests/lwt_helpers.h    | 139 ++++++++
 .../selftests/bpf/prog_tests/lwt_redirect.c   | 330 ++++++++++++++++++
 .../selftests/bpf/prog_tests/lwt_reroute.c    | 262 ++++++++++++++
 .../selftests/bpf/progs/test_lwt_redirect.c   |  90 +++++
 .../selftests/bpf/progs/test_lwt_reroute.c    |  36 ++
 10 files changed, 868 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_helpers.h
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_redirect.c
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_reroute.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_lwt_redirect.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_lwt_reroute.c

-- 
2.30.2

On 8/18/23 4:58 AM, Yan Zhai wrote:
> lwt xmit hook does not expect positive return values in function
> ip_finish_output2 and ip6_finish_output. However, BPF programs can
> directly return positive statuses such like NET_XMIT_DROP, NET_RX_DROP,
> and etc to the caller. Such return values would make the kernel continue
> processing already freed skbs and eventually panic.
> 
> This set fixes the return values from BPF ops to unexpected continue
> processing, checks strictly on the correct continue condition for
> future proof. In addition, add missing selftests for BPF redirect
> and reroute cases for BPF-CI.
> 
> v5: https://lore.kernel.org/bpf/cover.1692153515.git.yan@cloudflare.com/
> v4: https://lore.kernel.org/bpf/ZMD1sFTW8SFiex+x@debian.debian/T/
> v3: https://lore.kernel.org/bpf/cover.1690255889.git.yan@cloudflare.com/
> v2: https://lore.kernel.org/netdev/ZLdY6JkWRccunvu0@debian.debian/
> v1: https://lore.kernel.org/bpf/ZLbYdpWC8zt9EJtq@debian.debian/
> 
> changes since v5:
>   * fix BPF-CI failures due to missing config and busybox ping issue

Series looks good, thanks! Given we're fairly close to merge window and
this has been broken for quite some time, I took this into bpf-next.

Thanks,
Daniel

On Fri, Aug 18, 2023 at 9:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/18/23 4:58 AM, Yan Zhai wrote:
> > lwt xmit hook does not expect positive return values in function
> > ip_finish_output2 and ip6_finish_output. However, BPF programs can
> > directly return positive statuses such like NET_XMIT_DROP, NET_RX_DROP,
> > and etc to the caller. Such return values would make the kernel continue
> > processing already freed skbs and eventually panic.
> >
> > This set fixes the return values from BPF ops to unexpected continue
> > processing, checks strictly on the correct continue condition for
> > future proof. In addition, add missing selftests for BPF redirect
> > and reroute cases for BPF-CI.
> >
> > v5: https://lore.kernel.org/bpf/cover.1692153515.git.yan@cloudflare.com/
> > v4: https://lore.kernel.org/bpf/ZMD1sFTW8SFiex+x@debian.debian/T/
> > v3: https://lore.kernel.org/bpf/cover.1690255889.git.yan@cloudflare.com/
> > v2: https://lore.kernel.org/netdev/ZLdY6JkWRccunvu0@debian.debian/
> > v1: https://lore.kernel.org/bpf/ZLbYdpWC8zt9EJtq@debian.debian/
> >
> > changes since v5:
> >   * fix BPF-CI failures due to missing config and busybox ping issue
>
> Series looks good, thanks! Given we're fairly close to merge window and
> this has been broken for quite some time, I took this into bpf-next.
>
Thanks Daniel! Can you also queue this up for stable (or guide how I can do it)?

Yan


> Thanks,
> Daniel

On 8/18/23 6:01 PM, Yan Zhai wrote:
> On Fri, Aug 18, 2023 at 9:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>
>> On 8/18/23 4:58 AM, Yan Zhai wrote:
>>> lwt xmit hook does not expect positive return values in function
>>> ip_finish_output2 and ip6_finish_output. However, BPF programs can
>>> directly return positive statuses such like NET_XMIT_DROP, NET_RX_DROP,
>>> and etc to the caller. Such return values would make the kernel continue
>>> processing already freed skbs and eventually panic.
>>>
>>> This set fixes the return values from BPF ops to unexpected continue
>>> processing, checks strictly on the correct continue condition for
>>> future proof. In addition, add missing selftests for BPF redirect
>>> and reroute cases for BPF-CI.
>>>
>>> v5: https://lore.kernel.org/bpf/cover.1692153515.git.yan@cloudflare.com/
>>> v4: https://lore.kernel.org/bpf/ZMD1sFTW8SFiex+x@debian.debian/T/
>>> v3: https://lore.kernel.org/bpf/cover.1690255889.git.yan@cloudflare.com/
>>> v2: https://lore.kernel.org/netdev/ZLdY6JkWRccunvu0@debian.debian/
>>> v1: https://lore.kernel.org/bpf/ZLbYdpWC8zt9EJtq@debian.debian/
>>>
>>> changes since v5:
>>>    * fix BPF-CI failures due to missing config and busybox ping issue
>>
>> Series looks good, thanks! Given we're fairly close to merge window and
>> this has been broken for quite some time, I took this into bpf-next.
>>
> Thanks Daniel! Can you also queue this up for stable (or guide how I can do it)?

Given the Fixes tags, it will be picked up automatically once it lands in
Linus' tree.

Thanks,
Daniel

On Fri, Aug 18, 2023 at 11:08 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 8/18/23 6:01 PM, Yan Zhai wrote:
> > On Fri, Aug 18, 2023 at 9:55 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >>
> >> On 8/18/23 4:58 AM, Yan Zhai wrote:
> >>> lwt xmit hook does not expect positive return values in function
> >>> ip_finish_output2 and ip6_finish_output. However, BPF programs can
> >>> directly return positive statuses such like NET_XMIT_DROP, NET_RX_DROP,
> >>> and etc to the caller. Such return values would make the kernel continue
> >>> processing already freed skbs and eventually panic.
> >>>
> >>> This set fixes the return values from BPF ops to unexpected continue
> >>> processing, checks strictly on the correct continue condition for
> >>> future proof. In addition, add missing selftests for BPF redirect
> >>> and reroute cases for BPF-CI.
> >>>
> >>> v5: https://lore.kernel.org/bpf/cover.1692153515.git.yan@cloudflare.com/
> >>> v4: https://lore.kernel.org/bpf/ZMD1sFTW8SFiex+x@debian.debian/T/
> >>> v3: https://lore.kernel.org/bpf/cover.1690255889.git.yan@cloudflare.com/
> >>> v2: https://lore.kernel.org/netdev/ZLdY6JkWRccunvu0@debian.debian/
> >>> v1: https://lore.kernel.org/bpf/ZLbYdpWC8zt9EJtq@debian.debian/
> >>>
> >>> changes since v5:
> >>>    * fix BPF-CI failures due to missing config and busybox ping issue
> >>
> >> Series looks good, thanks! Given we're fairly close to merge window and
> >> this has been broken for quite some time, I took this into bpf-next.
> >>
> > Thanks Daniel! Can you also queue this up for stable (or guide how I can do it)?
>
> Given the Fixes tags, it will be picked up automatically once it lands in
> Linus' tree.
>
Wonderful. Thank you!

> Thanks,
> Daniel