netdev - Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results

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:   Fri, 19 Nov 2021 09:25:03 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Mauricio Vásquez <mauricio@...volk.io>
Cc:     Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Rafael David Tinoco <rafaeldtinoco@...il.com>,
        Lorenzo Fontana <lorenzo.fontana@...stic.co>,
        Leonardo Di Donato <leonardo.didonato@...stic.co>
Subject: Re: [PATCH bpf-next v2 4/4] libbpf: Expose CO-RE relocation results

On Tue, Nov 16, 2021 at 8:42 AM Mauricio Vásquez <mauricio@...volk.io> wrote:
>
> The result of the CO-RE relocations can be useful for some use cases
> like BTFGen[0]. This commit adds a new ‘record_core_relos’ option to
> save the result of such relocations and a couple of functions to access
> them.
>
> [0]: https://github.com/kinvolk/btfgen/
>
> Signed-off-by: Mauricio Vásquez <mauricio@...volk.io>
> Signed-off-by: Rafael David Tinoco <rafael.tinoco@...asec.com>
> Signed-off-by: Lorenzo Fontana <lorenzo.fontana@...stic.co>
> Signed-off-by: Leonardo Di Donato <leonardo.didonato@...stic.co>
> ---
>  tools/lib/bpf/libbpf.c    | 63 ++++++++++++++++++++++++++++++++++++++-
>  tools/lib/bpf/libbpf.h    | 49 +++++++++++++++++++++++++++++-
>  tools/lib/bpf/libbpf.map  |  2 ++
>  tools/lib/bpf/relo_core.c | 28 +++++++++++++++--
>  tools/lib/bpf/relo_core.h | 21 ++-----------
>  5 files changed, 140 insertions(+), 23 deletions(-)
>

Ok, I've meditated on this patch set long enough. I still don't like
that libbpf will be doing all this just for the sake of BTFGen's use
case.

In the end, I think giving bpftool access to internal APIs of libbpf
is more appropriate, and it seems like it's pretty easy to achieve. It
might actually clean up gen_loader parts a bit as well. So we'll need
to coordinate all this with Alexei's current work on CO-RE for kernel
as well.

But here's what I think could be done to keep libbpf internals simple.
We split bpf_core_apply_relo() into two parts: 1) calculating the
struct bpf_core_relo_res and 2) patching the instruction. If you look
at bpf_core_apply_relo, it needs prog just for prog_name (which we can
just pass everywhere for logging purposes) and to extract one specific
instruction to be patched. This instruction is passed at the very end
to bpf_core_patch_insn() after bpf_core_relo_res is calculated. So I
propose to make those two explicitly separate steps done by libbpf. So
bpf_core_apply_relo() (which we should rename to bpf_core_calc_relo()
or something like that) won't do any modification to the program
instructions. bpf_object__relocate_core() will do bpf_core_calc_relo()
first, if that's successful, it will pass the result into
bpf_core_patch_insn(). Simple and clean, unless I missed some
complication (happens all the time, but..)

At this point, we can teach bpftool to just take btf_ext (from BPF
object file), iterate over all CO-RE relos and call only
bpf_core_calc_relo() (no instruction patching). Using
bpf_core_relo_res bpftool will know types and fields that need to be
preserved and it will be able to construct minimal btf. So interface
for bpftool looks like this:

   bpftool gen distill_btf (or whatever the name) <file.bpf.o>
<distilled_raw.btf>

BTFGen as a solution, then, can use bpftool to process each pair of
btf + bpf object.

Thoughts?

[...]