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
| ||
|
Date: Fri, 25 Sep 2020 17:52:13 +0200 From: Daniel Borkmann <daniel@...earbox.net> To: Andrii Nakryiko <andrii.nakryiko@...il.com> Cc: Alexei Starovoitov <ast@...nel.org>, john fastabend <john.fastabend@...il.com>, Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org> Subject: Re: [PATCH bpf-next 4/6] bpf, libbpf: add bpf_tail_call_static helper for bpf programs On 9/25/20 5:42 PM, Daniel Borkmann wrote: > On 9/25/20 12:17 AM, Daniel Borkmann wrote: >> On 9/24/20 10:53 PM, Andrii Nakryiko wrote: >>> On Thu, Sep 24, 2020 at 11:22 AM Daniel Borkmann <daniel@...earbox.net> wrote: >>>> >>>> Port of tail_call_static() helper function from Cilium's BPF code base [0] >>>> to libbpf, so others can easily consume it as well. We've been using this >>>> in production code for some time now. The main idea is that we guarantee >>>> that the kernel's BPF infrastructure and JIT (here: x86_64) can patch the >>>> JITed BPF insns with direct jumps instead of having to fall back to using >>>> expensive retpolines. By using inline asm, we guarantee that the compiler >>>> won't merge the call from different paths with potentially different >>>> content of r2/r3. >>>> >>>> We're also using __throw_build_bug() macro in different places as a neat >>>> trick to trigger compilation errors when compiler does not remove code at >>>> compilation time. This works for the BPF backend as it does not implement >>>> the __builtin_trap(). >>>> >>>> [0] https://github.com/cilium/cilium/commit/f5537c26020d5297b70936c6b7d03a1e412a1035 >>>> >>>> Signed-off-by: Daniel Borkmann <daniel@...earbox.net> >>>> --- >>>> tools/lib/bpf/bpf_helpers.h | 32 ++++++++++++++++++++++++++++++++ >>>> 1 file changed, 32 insertions(+) >>>> >>>> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h >>>> index 1106777df00b..18b75a4c82e6 100644 >>>> --- a/tools/lib/bpf/bpf_helpers.h >>>> +++ b/tools/lib/bpf/bpf_helpers.h >>>> @@ -53,6 +53,38 @@ >>>> }) >>>> #endif >>>> >>>> +/* >>>> + * Misc useful helper macros >>>> + */ >>>> +#ifndef __throw_build_bug >>>> +# define __throw_build_bug() __builtin_trap() >>>> +#endif >>> >>> this will become part of libbpf stable API, do we want/need to expose >>> it? If we want to expose it, then we should probably provide a better >>> description. >>> >>> But also curious, how is it better than _Static_assert() (see >>> test_cls_redirect.c), which also allows to provide a better error >>> message? >> >> Need to get back to you whether that has same semantics. We use the __throw_build_bug() >> also in __bpf_memzero() and friends [0] as a way to trigger a hard build bug if we hit >> a default switch-case [0], so we detect unsupported sizes which are not covered by the >> implementation yet. If _Static_assert (0, "foo") does the trick, we could also use that; >> will check with our code base. > > So _Static_assert() won't work here, for example consider: > > # cat f1.c > int main(void) > { > if (0) > _Static_assert(0, "foo"); > return 0; > } > # clang -target bpf -Wall -O2 -c f1.c -o f1.o > f1.c:4:3: error: expected expression > _Static_assert(0, "foo"); > ^ > 1 error generated. .. aaand it looks like I need some more coffee. ;-) But result is the same after all: # clang -target bpf -Wall -O2 -c f1.c -o f1.o f1.c:4:3: error: static_assert failed "foo" _Static_assert(0, "foo"); ^ ~ 1 error generated. # cat f1.c int main(void) { if (0) { _Static_assert(0, "foo"); } return 0; } > In order for it to work as required form the use-case, the _Static_assert() must not trigger > here given the path is unreachable and will be optimized away. I'll add a comment to the > __throw_build_bug() helper. Given libbpf we should probably also prefix with bpf_. If you see > a better name that would fit, pls let me know. > >> [0] https://github.com/cilium/cilium/blob/master/bpf/include/bpf/builtins.h > Thanks, > Daniel
Powered by blists - more mailing lists