netdev - Re: bpf libraries and static variables. Was: [PATCH v2 bpf-next 2/6] libbpf: rename static variables during 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEf4BzYTgSTBf8+-Hqsv2SF-L17Tu9se=oZ=vEfekQY_+fLB=g@mail.gmail.com>
Date:   Tue, 11 May 2021 11:04:34 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Lorenz Bauer <lmb@...udflare.com>
Cc:     Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Yonghong Song <yhs@...com>,
        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>,
        John Fastabend <john.fastabend@...il.com>
Subject: Re: bpf libraries and static variables. Was: [PATCH v2 bpf-next 2/6]
 libbpf: rename static variables during linking

On Tue, May 11, 2021 at 7:20 AM Lorenz Bauer <lmb@...udflare.com> wrote:
>
> On Wed, 5 May 2021 at 06:22, Alexei Starovoitov
> <alexei.starovoitov@...il.com> wrote:
> >
> > > All of the above is up for discussion. I'd love to hear what golang folks
> > > are thinking, since above proposal is C centric.
>
> Sorry for the late reply, I was on holiday.
>
> Regarding your conntrack library example:
> - what is the difference between impl.bpf.c and ct_api.bpf.c? If I
> understand correctly, ct_api is used to generate the skel.h, but impl
> isn't?

I don't think it matters much, the point is that your BPF library can
be compiled from multiple .c files. Same for BPF application itself,
it can be compiled from multiple .c files and use multiple BPF
libraries.

> - what file would main.bpf.c include? ct_api or skel.h?

main.bpf.c will include (if at all) anything that BPF library will
provide *for BPF side of things*. I.e., some sort of ct_api.bpf.h.
skel.h is not supposed to be included in BPF code, only user-space.

>
> Regarding Andrii's proposal in the forwarded email to use __hidden,
> __internal etc. Are these correct:
> - static int foo: this is only available in the same .o, not
> accessible from user space. Can be referenced via extern int foo?

Yes about availability only from BPF and only within single .o. Not
true about extern, you can't extern statics (just like in user-space).

> - __hidden int foo: only available in same .o, not accessible from user space
> - __internal int foo: only available in same .a via extern, not
> accessible from user space

See my last RFC patch set for details (last patch especially with more
details in commit log). It's the other way around. __hidden means
available across multiple .o files during static linking. Once static
linking is done (e.g., you compiled BPF library into my_lib.bpf.o),
__hidden is restricted and converted into __internal. __internal means
not available outside of single .o. global __internal symbol is
equivalent to static variable, except it's accessible from BPF
skeleton/sub-skeleton and we have name uniqueness guarantee.

> - int foo: available / conflicts in all .o, accessible from user space
> (aka included in skel.h)

yes, it's a plain global symbol with STV_DEFAULT visibility.

>
> When you speak of the linker, do you mean libbpf or the clang / llvm
> linker? The Go toolchain has a simplistic linker to support bpf2bpf
> calls from the same .o so I imagine libbpf has something similar.

We are talking about libbpf's struct bpf_linker linker.

>
> > I want to clarify a few things that were brought up in offline discussions.
> > There are several options:
> > 1. don't emit statics at all.
> > That will break some skeleton users and doesn't solve the name conflict issue.
> > The library authors would need to be careful and use a unique enough
> > prefix for all global vars (including attribute("hidden") ones).
> > That's no different with traditional static linking in C.
> > bpf static linker already rejects linking if file1.bpf.c is trying to
> > 'extern int foo()'
> > when it was '__hidden int foo();' in file2.bpf.c
> > That's safer than traditional linker and the same approach can be
> > applied to vars.
> > So externing of __hidden vars won't be possible, but they will name conflict.
> >

[...]

>
> --
> Lorenz Bauer  |  Systems Engineer
> 6th Floor, County Hall/The Riverside Building, SE1 7PB, UK
>
> www.cloudflare.com