netdev - Re: [PATCH bpf-next 7/9] libbpf: add BTF writing APIs

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: <CAEf4BzYFswwhJOq4bSDZU5-bqUo+LwwUQ_NRH_zkBgGcBVYOjw@mail.gmail.com>
Date:   Thu, 24 Sep 2020 23:21:44 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Kernel Team <kernel-team@...com>,
        Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH bpf-next 7/9] libbpf: add BTF writing APIs

On Thu, Sep 24, 2020 at 8:55 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Wed, Sep 23, 2020 at 08:54:34AM -0700, Andrii Nakryiko wrote:
> > Add APIs for appending new BTF types at the end of BTF object.
> >
> > Each BTF kind has either one API of the form btf__append_<kind>(). For types
> > that have variable amount of additional items (struct/union, enum, func_proto,
> > datasec), additional API is provided to emit each such item. E.g., for
> > emitting a struct, one would use the following sequence of API calls:
> >
> > btf__append_struct(...);
> > btf__append_field(...);
> > ...
> > btf__append_field(...);
>
> I've just started looking through the diffs. The first thing that struck me
> is the name :) Why 'append' instead of 'add' ? The latter is shorter.

Append is very precise about those types being added at the end. Add
is more ambiguous in that sense and doesn't imply any specific order.
E.g., for btf__add_str() that's suitable, because the order in which
strings are inserted might be different (e.g., if the string is
duplicated). But it's not an "insert" either, so I'm fine with
renaming to "add", if you prefer it.

>
> Also how would you add anon struct that is within another struct ?
> The anon one would have to be added first and then added as a field?
> Feels a bit odd that struct/union building doesn't have 'finish' method,
> but I guess it can work.

That embedded anon struct will be a separate type, then the field
(anonymous or not, that's orthogonal to anonymity of a struct (!))
will refer to that anon struct type by its ID. Anon struct can be
added right before, right after, or in between completely unrelated
types, it doesn't matter to BTF itself as long as all the type IDs
match in the end.

As for the finish method... There wasn't a need so far to have it, as
BTF constantly maintains correct vlen for the "current"
struct/union/func_proto/datasec/enum (anything with extra members).
I've been writing a few more tests than what I posted here (they will
come in the second wave of patches) and converted pahole to this new
API completely. And it does feel pretty nice and natural so far. In
the future we might add something like that, I suppose, that would
allow to do some more validations at the end. But that would be a
non-breaking extension, so I don't want to do it right now.