netdev - Re: [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during static linking

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] [day] [month] [year] [list]

Message-ID: <CAEf4BzZSnQyiE+y=5eK3110hxtug_YUSnH0BPjEjzEtg19zYpQ@mail.gmail.com>
Date:   Thu, 25 Mar 2021 21:17:02 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Yonghong Song <yhs@...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>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH bpf-next] libbpf: preserve empty DATASEC BTFs during
 static linking

On Thu, Mar 25, 2021 at 8:31 AM Yonghong Song <yhs@...com> wrote:
>
>
>
> On 3/24/21 10:11 PM, Andrii Nakryiko wrote:
> > Ensure that BPF static linker preserves all DATASEC BTF types, even if some of
> > them might not have any variable information at all. It's not completely clear
> > in which cases Clang chooses to not emit variable information, so adding
> > reliable repro is hard. But manual testing showed that this work correctly.
>
> This may happen if the compiler promotes local initialized variable
> contents into .rodata section and there are no global or static
> functions in the program.
>
> For example,
> $ cat t.c
> struct t { char a; char b; char c; };
> void bar(struct t*);
> void find() {
>    struct t tmp = {1, 2, 3};
>    bar(&tmp);
> }
>
> $ clang -target bpf -O2 -g -S t.c
> you will find:
>
>          .long   104                             # BTF_KIND_DATASEC(id = 8)
>          .long   251658240                       # 0xf000000
>          .long   0
>
>          .ascii  ".rodata"                       # string offset=104
>
> $ clang -target bpf -O2 -g -c t.c
> $ readelf -S t.o | grep data
>    [ 4] .rodata           PROGBITS         0000000000000000  00000090
>
> >
> > Reported-by: Alexei Starovoitov <ast@...nel.org>
> > Fixes: 8fd27bf69b86 ("libbpf: Add BPF static linker BTF and BTF.ext support")
> > Signed-off-by: Andrii Nakryiko <andrii@...nel.org>
>
> Ack with a nit below.
>
> Acked-by: Yonghong Song <yhs@...com>
>
> > ---
> >   tools/lib/bpf/linker.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/lib/bpf/linker.c b/tools/lib/bpf/linker.c
> > index 5e0aa2f2c0ca..2c43943da30c 100644
> > --- a/tools/lib/bpf/linker.c
> > +++ b/tools/lib/bpf/linker.c
> > @@ -94,6 +94,7 @@ struct dst_sec {
> >       int sec_sym_idx;
> >
> >       /* section's DATASEC variable info, emitted on BTF finalization */
> > +     bool has_btf;
> >       int sec_var_cnt;
> >       struct btf_var_secinfo *sec_vars;
> >
> > @@ -1436,6 +1437,15 @@ static int linker_append_btf(struct bpf_linker *linker, struct src_obj *obj)
> >                       continue;
> >               dst_sec = &linker->secs[src_sec->dst_id];
> >
> > +             /* Mark section as having BTF regardless of the presence of
> > +              * variables. It seems to happen sometimes when BPF object
> > +              * file has only static variables inside functions, not
> > +              * globally, that DATASEC BTF with zero variables will be
> > +              * emitted by Clang. We need to preserve such empty BTF and
>
> Maybe give a more specific example here, e.g.,
> For example, these static variables may be generated by the compiler
> by promoting local array/structure variable initial values.

Yep, thanks for explanations, I'll update the comment and commit message.

>
> > +              * just set correct section size.
> > +              */
> > +             dst_sec->has_btf = true;
> > +
> >               t = btf__type_by_id(obj->btf, src_sec->sec_type_id);
> >               src_var = btf_var_secinfos(t);
> >               n = btf_vlen(t);
> > @@ -1717,7 +1727,7 @@ static int finalize_btf(struct bpf_linker *linker)
> >       for (i = 1; i < linker->sec_cnt; i++) {
> >               struct dst_sec *sec = &linker->secs[i];
> >
> > -             if (!sec->sec_var_cnt)
> > +             if (!sec->has_btf)
> >                       continue;
> >
> >               id = btf__add_datasec(btf, sec->sec_name, sec->sec_sz);
> >