netdev - Re: [PATCH bpf-next 2/9] libbpf: remove assumption of single contiguous memory for BTF data

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: <CAEf4BzYV+f2JXJDMvbNL0f6GQDPMaOOmsM7M=pO=f4mF2RfUig@mail.gmail.com>
Date:   Thu, 24 Sep 2020 13:25:21 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     John Fastabend <john.fastabend@...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 2/9] libbpf: remove assumption of single
 contiguous memory for BTF data

On Thu, Sep 24, 2020 at 8:21 AM John Fastabend <john.fastabend@...il.com> wrote:
>
> Andrii Nakryiko wrote:
> > Refactor internals of struct btf to remove assumptions that BTF header, type
> > data, and string data are layed out contiguously in a memory in a single
> > memory allocation. Now we have three separate pointers pointing to the start
> > of each respective are: header, types, strings. In the next patches, these
> > pointers will be re-assigned to point to independently allocated memory areas,
> > if BTF needs to be modified.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
>
> [...]
>
> > --- a/tools/lib/bpf/btf.c
> > +++ b/tools/lib/bpf/btf.c
> > @@ -27,18 +27,37 @@
> >  static struct btf_type btf_void;
> >
> >  struct btf {
> > -     union {
> > -             struct btf_header *hdr;
> > -             void *data;
> > -     };
> > +     void *raw_data;
> > +     __u32 raw_size;
> > +
> > +     /*
> > +      * When BTF is loaded from ELF or raw memory it is stored
> > +      * in contiguous memory block, pointed to by raw_data pointer, and
> > +      * hdr, types_data, and strs_data point inside that memory region to
> > +      * respective parts of BTF representation:
>
> I find the above comment a bit confusing. The picture though is great. How
> about something like,
>
>   When BTF is loaded from an ELF or raw memory it is stored
>   in a continguous memory block. The hdr, type_data, and strs_data
>   point inside that memory region to their respective parts of BTF
>   representation
>

Had to do a mental diff to find the part you didn't like :) I'll
update the comment as you suggested.

> > +      *
> > +      * +--------------------------------+
> > +      * |  Header  |  Types  |  Strings  |
> > +      * +--------------------------------+
> > +      * ^          ^         ^
> > +      * |          |         |
> > +      * hdr        |         |
> > +      * types_data-+         |
> > +      * strs_data------------+
> > +      */
> > +     struct btf_header *hdr;
> > +     void *types_data;
> > +     void *strs_data;
> > +
> > +     /* type ID to `struct btf_type *` lookup index */
> >       __u32 *type_offs;
> >       __u32 type_offs_cap;
> > -     const char *strings;
> > -     void *nohdr_data;
> > -     void *types_data;
> >       __u32 nr_types;
> > -     __u32 data_size;
> > +
> > +     /* BTF object FD, if loaded into kernel */
> >       int fd;
> > +
> > +     /* Pointer size (in bytes) for a target architecture of this BTF */
> >       int ptr_sz;
> >  };
> >
>
> Thanks,
> John