netdev - Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Feb 2018 11:59:29 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Jesper Dangaard Brouer <brouer@...hat.com>, netdev@...r.kernel.org,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        wangnan0@...wei.com
Cc:     acme@...hat.com, joe@....org, jakub.kicinski@...ronome.com,
        eric@...it.org
Subject: Re: [bpf-next PATCH 0/5] tools/libbpf improvements and selftests

Hi Jesper,

On 01/27/2018 06:26 PM, Jesper Dangaard Brouer wrote:
> While playing with using libbpf for the Suricata project, we had
> issues LLVM >= 4.0.1 generating ELF files that could not be loaded
> with libbpf (tools/lib/bpf/).
> 
> During the troubleshooting phase, I wrote a test program and improved
> the debugging output in libbpf.  I turned this into a selftests
> program, and it also serves as a code example for libbpf in itself.
> 
> I discovered that there are at least three ELF load issues with
> libbpf.  I left them as TODO comments in (tools/testing/selftests/bpf)
> test_libbpf.sh. I've only fixed the load issue with eh_frames.  We can
> work on the other issues later.

The series looks great, and given the fix, we should get this into bpf.
Only one small request, could you move the test_libbpf_open.c into the
BPF selftests as well? We have test_libbpf.sh there which is the only
one making use of test_libbpf_open.c, so I think it fits much better if
we put them both into selftests. Otherwise rest is fine, thanks!

Regarding the TODO comments:

+# TODO: fix libbpf to load noinline functions
+# [warning] libbpf: incorrect bpf_call opcode
+#libbpf_open_file test_l4lb_noinline.o

Right, so this would require a newer llvm version that supports calls.
Maybe there could be a fallback to turn the noinline into __always_inline
for older llvms with dumping a warning to the user that this case cannot
properly be tested due to old llvm version.

+# TODO: fix test_xdp_meta.c to load with libbpf
+# [warning] libbpf: test_xdp_meta.o doesn't provide kernel version
+#libbpf_open_file test_xdp_meta.o

The kernel version is only required for tracing programs, although
it's just fine as well to put a dummy 'int _version SEC("version") = 1;'
here. The test_xdp_meta.sh uses iproute2 for loading into the kernel,
but no objections to add a version section there.

+# TODO: fix libbpf to handle .eh_frame
+# [warning] libbpf: relocation failed: no section(10)
+#libbpf_open_file ../../../../samples/bpf/tracex3_kern.o

This is resolved in your last patch then, right? So the two could just
be swapped and this one uncommented, although it would require that
tracex3_kern.o has been built earlier.

Thanks,
Daniel

> Jesper Dangaard Brouer (5):
>       bpf: Sync kernel ABI header with tooling header for bpf_common.h
>       tools/libbpf: improve the pr_debug statements to contain section numbers
>       tools/libbpf: add test program for loading BPF ELF files
>       selftests/bpf: add selftest that use test_libbpf_open
>       tools/libbpf: handle issues with bpf ELF objects containing .eh_frames
> 
> 
>  tools/testing/selftests/bpf/Makefile       |    9 +++++-
>  tools/testing/selftests/bpf/test_libbpf.sh |   45 ++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+), 1 deletion(-)
>  create mode 100755 tools/testing/selftests/bpf/test_libbpf.sh
> 
> --
>