mbox series

[v5,bpf,0/4] lwt: fix return values of BPF ops

Message ID cover.1692153515.git.yan@cloudflare.com (mailing list archive)
Headers show
Series lwt: fix return values of BPF ops | expand

Message

Yan Zhai Aug. 16, 2023, 2:54 a.m. UTC 
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, and checks strictly on the correct continue condition for
future proof. In addition, add missing selftests for BPF_REDIRECT
and BPF_REROUTE cases for BPF-CI.

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 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 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 +-
 .../selftests/bpf/prog_tests/lwt_helpers.h    | 139 ++++++++
 .../selftests/bpf/prog_tests/lwt_redirect.c   | 319 ++++++++++++++++++
 .../selftests/bpf/prog_tests/lwt_reroute.c    | 256 ++++++++++++++
 .../selftests/bpf/progs/test_lwt_redirect.c   |  58 ++++
 .../selftests/bpf/progs/test_lwt_reroute.c    |  36 ++
 9 files changed, 817 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

Comments

Daniel Borkmann Aug. 16, 2023, 2:27 p.m. UTC | #1 
Hi Yan,

On 8/16/23 4:54 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, and checks strictly on the correct continue condition for
> future proof. In addition, add missing selftests for BPF_REDIRECT
> and BPF_REROUTE cases for BPF-CI.
> 
> 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 v4:
>   * fixed same error on BPF_REROUTE path
>   * re-implemented selftests under BPF-CI requirement

BPF CI failed: https://github.com/kernel-patches/bpf/actions/runs/5874202507/job/15929012788

Looks like due to dummy device issue. Either you might need to add this to
the tools/testing/selftests/bpf/config* or perhaps just use veth instead for
link_err dev.

Error from the above link:

Notice: Success: 370/3177, Skipped: 21, Failed: 2
Error: #131 lwt_redirect
   Error: #131 lwt_redirect
   test_lwt_redirect:PASS:pthread_create 0 nsec
Error: #131/1 lwt_redirect/lwt_redirect_normal
   Error: #131/1 lwt_redirect/lwt_redirect_normal
   test_lwt_redirect_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_redirect_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup_redirect_target:PASS:open_tuntap 0 nsec
   setup_redirect_target:PASS:if_nametoindex 0 nsec
   setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   test_lwt_redirect_normal:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
Error: #131/2 lwt_redirect/lwt_redirect_normal_nomac
   Error: #131/2 lwt_redirect/lwt_redirect_normal_nomac
   test_lwt_redirect_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_redirect_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup_redirect_target:PASS:open_tuntap 0 nsec
   setup_redirect_target:PASS:if_nametoindex 0 nsec
   setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   test_lwt_redirect_normal_nomac:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
Error: #131/3 lwt_redirect/lwt_redirect_dev_down
   Error: #131/3 lwt_redirect/lwt_redirect_dev_down
   test_lwt_redirect_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_redirect_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup_redirect_target:PASS:open_tuntap 0 nsec
   setup_redirect_target:PASS:if_nametoindex 0 nsec
   setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   __test_lwt_redirect_dev_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
Error: #131/4 lwt_redirect/lwt_redirect_dev_down_nomac
   Error: #131/4 lwt_redirect/lwt_redirect_dev_down_nomac
   test_lwt_redirect_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_redirect_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup_redirect_target:PASS:open_tuntap 0 nsec
   setup_redirect_target:PASS:if_nametoindex 0 nsec
   setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   __test_lwt_redirect_dev_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
Error: #131/5 lwt_redirect/lwt_redirect_dev_carrier_down
   Error: #131/5 lwt_redirect/lwt_redirect_dev_carrier_down
   test_lwt_redirect_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_redirect_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup_redirect_target:PASS:open_tuntap 0 nsec
   setup_redirect_target:PASS:if_nametoindex 0 nsec
   setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   test_lwt_redirect_dev_carrier_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
   test_lwt_redirect:PASS:pthread_join 0 nsec
Error: #132 lwt_reroute
   Error: #132 lwt_reroute
   test_lwt_reroute:PASS:pthread_create 0 nsec
Error: #132/1 lwt_reroute/lwt_reroute_normal_xmit
   Error: #132/1 lwt_reroute/lwt_reroute_normal_xmit
   test_lwt_reroute_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_reroute_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup:PASS:open_tun 0 nsec
   setup:PASS:if_nametoindex 0 nsec
   setup:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   test_lwt_reroute_normal_xmit:FAIL:setup_reroute unexpected setup_reroute: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
Error: #132/2 lwt_reroute/lwt_reroute_qdisc_dropped
   Error: #132/2 lwt_reroute/lwt_reroute_qdisc_dropped
   test_lwt_reroute_run:PASS:netns_create 0 nsec
   open_netns:PASS:malloc token 0 nsec
   open_netns:PASS:open /proc/self/ns/net 0 nsec
   open_netns:PASS:open netns fd 0 nsec
   open_netns:PASS:setns 0 nsec
   test_lwt_reroute_run:PASS:setns 0 nsec
   open_tuntap:PASS:open(/dev/net/tun) 0 nsec
   open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
   open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
   setup:PASS:open_tun 0 nsec
   setup:PASS:if_nametoindex 0 nsec
   setup:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
   test_lwt_reroute_qdisc_dropped:FAIL:setup_reroute unexpected setup_reroute: actual -1 < expected 0
   close_netns:PASS:setns 0 nsec
   test_lwt_reroute:PASS:pthread_join 0 nsec
Test Results:
              bpftool: PASS
           test_progs: FAIL (returned 1)
             shutdown: CLEAN
Error: Process completed with exit code 1.

Thanks,
Daniel
Yan Zhai Aug. 16, 2023, 3:21 p.m. UTC | #2 
On Wed, Aug 16, 2023 at 9:27 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Hi Yan,
>
> On 8/16/23 4:54 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, and checks strictly on the correct continue condition for
> > future proof. In addition, add missing selftests for BPF_REDIRECT
> > and BPF_REROUTE cases for BPF-CI.
> >
> > 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 v4:
> >   * fixed same error on BPF_REROUTE path
> >   * re-implemented selftests under BPF-CI requirement
>
> BPF CI failed: https://github.com/kernel-patches/bpf/actions/runs/5874202507/job/15929012788
>
> Looks like due to dummy device issue. Either you might need to add this to
> the tools/testing/selftests/bpf/config* or perhaps just use veth instead for
> link_err dev.
>
It is indeed the dummy driver issue. I will update the config. Thanks
for notifying me, now I know where to look at build results.

Yan

> Error from the above link:
>
> Notice: Success: 370/3177, Skipped: 21, Failed: 2
> Error: #131 lwt_redirect
>    Error: #131 lwt_redirect
>    test_lwt_redirect:PASS:pthread_create 0 nsec
> Error: #131/1 lwt_redirect/lwt_redirect_normal
>    Error: #131/1 lwt_redirect/lwt_redirect_normal
>    test_lwt_redirect_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_redirect_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup_redirect_target:PASS:open_tuntap 0 nsec
>    setup_redirect_target:PASS:if_nametoindex 0 nsec
>    setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    test_lwt_redirect_normal:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
> Error: #131/2 lwt_redirect/lwt_redirect_normal_nomac
>    Error: #131/2 lwt_redirect/lwt_redirect_normal_nomac
>    test_lwt_redirect_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_redirect_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup_redirect_target:PASS:open_tuntap 0 nsec
>    setup_redirect_target:PASS:if_nametoindex 0 nsec
>    setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    test_lwt_redirect_normal_nomac:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
> Error: #131/3 lwt_redirect/lwt_redirect_dev_down
>    Error: #131/3 lwt_redirect/lwt_redirect_dev_down
>    test_lwt_redirect_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_redirect_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup_redirect_target:PASS:open_tuntap 0 nsec
>    setup_redirect_target:PASS:if_nametoindex 0 nsec
>    setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    __test_lwt_redirect_dev_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
> Error: #131/4 lwt_redirect/lwt_redirect_dev_down_nomac
>    Error: #131/4 lwt_redirect/lwt_redirect_dev_down_nomac
>    test_lwt_redirect_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_redirect_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup_redirect_target:PASS:open_tuntap 0 nsec
>    setup_redirect_target:PASS:if_nametoindex 0 nsec
>    setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    __test_lwt_redirect_dev_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
> Error: #131/5 lwt_redirect/lwt_redirect_dev_carrier_down
>    Error: #131/5 lwt_redirect/lwt_redirect_dev_carrier_down
>    test_lwt_redirect_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_redirect_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup_redirect_target:PASS:open_tuntap 0 nsec
>    setup_redirect_target:PASS:if_nametoindex 0 nsec
>    setup_redirect_target:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    test_lwt_redirect_dev_carrier_down:FAIL:setup_redirect_target unexpected setup_redirect_target: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
>    test_lwt_redirect:PASS:pthread_join 0 nsec
> Error: #132 lwt_reroute
>    Error: #132 lwt_reroute
>    test_lwt_reroute:PASS:pthread_create 0 nsec
> Error: #132/1 lwt_reroute/lwt_reroute_normal_xmit
>    Error: #132/1 lwt_reroute/lwt_reroute_normal_xmit
>    test_lwt_reroute_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_reroute_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup:PASS:open_tun 0 nsec
>    setup:PASS:if_nametoindex 0 nsec
>    setup:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    test_lwt_reroute_normal_xmit:FAIL:setup_reroute unexpected setup_reroute: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
> Error: #132/2 lwt_reroute/lwt_reroute_qdisc_dropped
>    Error: #132/2 lwt_reroute/lwt_reroute_qdisc_dropped
>    test_lwt_reroute_run:PASS:netns_create 0 nsec
>    open_netns:PASS:malloc token 0 nsec
>    open_netns:PASS:open /proc/self/ns/net 0 nsec
>    open_netns:PASS:open netns fd 0 nsec
>    open_netns:PASS:setns 0 nsec
>    test_lwt_reroute_run:PASS:setns 0 nsec
>    open_tuntap:PASS:open(/dev/net/tun) 0 nsec
>    open_tuntap:PASS:ioctl(TUNSETIFF) 0 nsec
>    open_tuntap:PASS:fcntl(O_NONBLOCK) 0 nsec
>    setup:PASS:open_tun 0 nsec
>    setup:PASS:if_nametoindex 0 nsec
>    setup:FAIL:ip link add link_err type dummy unexpected error: 512 (errno 0)
>    test_lwt_reroute_qdisc_dropped:FAIL:setup_reroute unexpected setup_reroute: actual -1 < expected 0
>    close_netns:PASS:setns 0 nsec
>    test_lwt_reroute:PASS:pthread_join 0 nsec
> Test Results:
>               bpftool: PASS
>            test_progs: FAIL (returned 1)
>              shutdown: CLEAN
> Error: Process completed with exit code 1.
>
> Thanks,
> Daniel