Re: [PATCH bpf-next] libbpf: support module BTF for BPF_TYPE_ID_TARGET CO-RE relocation

On Sat, Dec 5, 2020 at 4:38 PM Alan Maguire [off-list ref] wrote:

On Fri, 4 Dec 2020, Andrii Nakryiko wrote:

quoted

When Clang emits ldimm64 instruction for BPF_TYPE_ID_TARGET CO-RE relocation,
put module BTF FD, containing target type, into upper 32 bits of imm64.

Because this FD is internal to libbpf, it's very cumbersome to test this in
selftests. Manual testing was performed with debug log messages sprinkled
across selftests and libbpf, confirming expected values are substituted.
Better testing will be performed as part of the work adding module BTF types
support to  bpf_snprintf_btf() helpers.

Cc: Alan Maguire <redacted>
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Thanks so much for doing this Andrii! When I tested, I ran into a problem;
it turns out when a module struct such as "veth_stats" is used, it's
classified as BTF_KIND_FWD, and as a result when we iterate over
the modules and look in the veth module, "struct veth_stats" does not
match since its module kind (BTF_KIND_STRUCT) does not match the candidate
kind (BTF_KIND_FWD). I'm kind of out of my depth here, but the below
patch (on top of your patch) worked.

I'm not quite clear on the situation. BTF_KIND_FWD is for the local
type or the remote type? Maybe a small example would help, before we
go straight to assuming FWD can be always resolved into a concrete
STRUCT/UNION.

 However without it - when we find
0  candidate matches - as well as not substituting the module object
id/type id - we hit a segfault:

Yep, I missed the null check in:

targ_spec->btf != prog->obj->btf_vmlinux

I'll fix that.

Program terminated with signal 11, Segmentation fault.
#0  0x0000000000480bf9 in bpf_core_calc_relo (prog=0x4d6ba40,
relo=0x4d70e7c,
    relo_idx=0, local_spec=0x7ffe2cf17b00, targ_spec=0x0,
res=0x7ffe2cf17ae0)
    at libbpf.c:4408
4408            switch (kind) {
Missing separate debuginfos, use: debuginfo-install
elfutils-libelf-0.172-2.el7.x86_64 glibc-2.17-196.el7.x86_64
libattr-2.4.46-13.el7.x86_64 libcap-2.22-9.el7.x86_64
libgcc-4.8.5-36.0.1.el7_6.2.x86_64 zlib-1.2.7-18.el7.x86_64
(gdb) bt
#0  0x0000000000480bf9 in bpf_core_calc_relo (prog=0x4d6ba40,
relo=0x4d70e7c,
    relo_idx=0, local_spec=0x7ffe2cf17b00, targ_spec=0x0,
res=0x7ffe2cf17ae0)
    at libbpf.c:4408


The dereferences of targ_spec in bpf_core_recalc_relo() seem
to be the cause; that function is called with a NULL targ_spec
when 0 candidates are found, so it's possible we'd need to
guard those accesses for cases where a bogus type was passed
in and no candidates were found.  If the below looks good would
it make sense to roll it into your patch or will I add it to my
v3 patch series?

Thanks again for your help with this!

Alan

From 08040730dbff6c5d7636927777ac85a71c10827f Mon Sep 17 00:00:00 2001
From: Alan Maguire <redacted>
Date: Sun, 6 Dec 2020 01:10:28 +0100
Subject: [PATCH] libbpf: handle fwd kinds when checking candidate relocations
 for modules

when a struct belonging to a module is being assessed, it will be
designated a fwd kind (BTF_KIND_FWD); when matching candidate
types constraints on exact type matching need to be relaxed to
ensure that such structures are found successfully.  Introduce
kinds_match() function to handle this comparison.

Signed-off-by: Alan Maguire <redacted>
---
 tools/lib/bpf/libbpf.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

[...]