netdev - Re: [PATCH bpf-next 3/3] selftests/bpf: allow compiling BPF objects without BTF

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 26 Mar 2021 09:44:07 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next 3/3] selftests/bpf: allow compiling BPF objects
 without BTF

On Mon, Mar 22, 2021 at 10:54 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Mon, Mar 22, 2021 at 09:56:19AM -0700, Andrii Nakryiko wrote:
> > On Sun, Mar 21, 2021 at 6:07 PM Alexei Starovoitov
> > <alexei.starovoitov@...il.com> wrote:
> > >
> > > On Sat, Mar 20, 2021 at 10:00:57AM -0700, Andrii Nakryiko wrote:
> > > > On Fri, Mar 19, 2021 at 7:22 PM Alexei Starovoitov
> > > > <alexei.starovoitov@...il.com> wrote:
> > > > >
> > > > > On Fri, Mar 19, 2021 at 01:59:09PM -0700, Andrii Nakryiko wrote:
> > > > > > Add ability to skip BTF generation for some BPF object files. This is done
> > > > > > through using a convention of .nobtf.c file name suffix.
> > > > > >
> > > > > > Also add third statically linked file to static_linked selftest. This file has
> > > > > > no BTF, causing resulting object file to have only some of DATASEC BTF types.
> > > > > > It also is using (from BPF code) global variables. This tests both libbpf's
> > > > > > static linking logic and bpftool's skeleton generation logic.
> > > > >
> > > > > I don't like the long term direction of patch 1 and 3.
> > > > > BTF is mandatory for the most bpf kernel features added in the last couple years.
> > > > > Making user space do quirks for object files without BTF is not something
> > > > > we should support or maintain. If there is no BTF the linker and skeleton
> > > > > generation shouldn't crash, of course, but they should reject such object.
> > > >
> > > > I don't think tools need to enforce any policies like that. They are
> > > > tools and should be unassuming about the way they are going to be used
> > > > to the extent possible.
> > >
> > > Right and bpftool/skeleton was used with BTF since day one.
> > > Without BTF the skeleton core ideas are lost. The skeleton api
> > > gives no benefit. So what's the point of adding support for skeleton without BTF?
> > > Is there a user that would benefit? If so, what will they gain from
> > > such BTF-less skeleton?
> >
> > The only part of skeleton API that's not available is convenient
> > user-space access to global variables. If you don't use global
> > variables you don't use BTF at all with skeleton. So all features but
> > one work without BTF just fine: compile-time maps and progs (and
> > links) references, embedding object file in .skel.h, and even
> > automatic memory-mapping of .data/.rodata/.bss (just unknown struct
> > layout).
> >
> > Compile-time maps and progs and separately object file embedding in C
> > header are useful in their own rights, even individually. There is no
> > single "core idea" of the BPF skeleton in my mind. What is it for you?
> >
> > So given none of the fixes are horrible hacks and won't incur
> > additional maintenance costs, what's the problem with accepting them?
>
> Because they double the maintenance cost now and double the support forever.
> We never needed to worry about skeleton without BTF and now it would be
> a thing ? So all tests realistically need to be doubled: with and without BTF.

2x? Realistically?.. No, I wouldn't say so. :) Extra test or a few
might be warranted, but doubling the amount of testing is a huge
exaggeration.

> Even more so for static linking. If one .o has BTF and another doesn't
> what linker suppose to do? Keep it, but the linked BTF will sort of cover
> both .o-s, but line info in some funcs will be missing.
> All these weird combinations would need to be tested.

BPF static linker already supports that mode, btw. And yes, it
shouldn't crash the kernel. And you don't need a skeleton or static
linker to do that to the kernel, so I don't know how that is a new
mode of operation.

> The sensible thing to do would be to reject skel generation without BTF
> and reject linking without BTF. The user most likely forgot -g during
> compilation of bpf prog. The bpftool should give the helpful message
> in such case. Whether it's generating skel or linking. Silently proceeding
> and generating half-featured skeleton is not what user wanted.

Sure, a warning message makes sense. Outright disabling this - not so
much. I still can't get why I can't get BPF skeleton and static
linking without BTF, if I really want to. Both are useful without BTF.

So I don't know, it's the third different argument I'm addressing
without any conclusion on the previous two. It, sadly, feels rather
like fighting subjective personal preferences, rather than a
discussion.