On Sat, Dec 5, 2020 at 4:38 PM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote:
>
> On Fri, 4 Dec 2020, Andrii Nakryiko wrote:
>
> > 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 <alan.maguire@xxxxxxxxxx>
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> 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 <alan.maguire@xxxxxxxxxx>
> 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 <alan.maguire@xxxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 24 +++++++++++++++++++++---
> 1 file changed, 21 insertions(+), 3 deletions(-)
>
[...]
