netdev - Re: [PATCH bpf] tools/bpftool: Add/Fix support for modules btf dump

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]

Message-ID: <76aa0d16e3d03cf12496184c848f60069bf71872.camel@kernel.org>
Date:   Mon, 07 Dec 2020 22:26:50 -0800
From:   Saeed Mahameed <saeed@...nel.org>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Networking <netdev@...r.kernel.org>,
        Andrii Nakryiko <andrii@...nel.org>
Subject: Re: [PATCH bpf] tools/bpftool: Add/Fix support for modules btf dump

On Mon, 2020-12-07 at 19:14 -0800, Andrii Nakryiko wrote:
> On Sun, Dec 6, 2020 at 9:21 PM <saeed@...nel.org> wrote:
> > From: Saeed Mahameed <saeedm@...dia.com>
> > 
> > While playing with BTF for modules, i noticed that executing the
> > command:
> > $ bpftool btf dump id <module's btf id>
> > 
> > Fails due to lack of information in the BTF data.
> > 
> > Maybe I am missing a step but actually adding the support for this
> > is
> > very simple.
> 
> yes, bpftool takes -B <path> argument for specifying base BTF. So if
> you added -B /sys/kernel/btf/vmlinux, it should have worked. I've
> added auto-detection logic for the case of `btf dump file
> /sys/kernel/btf/<module>` (see [0]), and we can also add it for when
> ID corresponds to a module BTF. But I think it's simplest to re-use
> the logic and just open /sys/kernel/btf/vmlinux, instead of adding
> narrowly-focused libbpf API for that.
> 

When dumping with a file it works even without the -B since you lookup
the vmlinux file, but the issue is not dumping from a file source, we
need it by id..

> > To completely parse modules BTF data, we need the vmlinux BTF as
> > their
> > "base btf", which can be easily found by iterating through the btf
> > ids and
> > looking for btf.name == "vmlinux".
> > 
> > I am not sure why this hasn't been added by the original patchset
> 
> because I never though of dumping module BTF by id, given there is
> nicely named /sys/kernel/btf/<module> :)
> 

What if i didn't compile my kernel with SYSFS ? a user experience is a
user experience, there is no reason to not support dump a module btf by
id or to have different behavior for different BTF sources.

I can revise this patch to support -B option and lookup vmlinux file if
not provided for module btf dump by ids.

but we  still need to pass base_btf to btf__get_from_id() in order to
support that, as was done for btf__parse_split() ... :/

Are you sure you don't like the current patch/libbpf API ? it is pretty
straight forward and correct.

> > "Integrate kernel module BTF support", as adding the support for
> > this is very trivial. Unless i am missing something, CCing Andrii..
> > 
> > Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
> > CC: Andrii Nakryiko <andrii@...nel.org>
> > ---
> >  tools/lib/bpf/btf.c      | 57
> > ++++++++++++++++++++++++++++++++++++++++
> >  tools/lib/bpf/btf.h      |  2 ++
> >  tools/lib/bpf/libbpf.map |  1 +
> >  3 files changed, 60 insertions(+)
> > 
> 
> [...]