netdev - Re: [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: <20210426223449.5njjmcjpu63chqbb@ast-mbp.dhcp.thefacebook.com>
Date:   Mon, 26 Apr 2021 15:34:49 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     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>
Subject: Re: [PATCH v2 bpf-next 2/6] libbpf: rename static variables during
 linking

On Mon, Apr 26, 2021 at 08:44:04AM -0700, Andrii Nakryiko wrote:
> 
> >
> > > Static maps are slightly different, because we use SEC() which marks
> > > them as used, so they should always be present.
> >
> > yes. The used attribute makes the compiler keep the data,
> > but it can still inline it and lose the reference in the .text.
> 
> At least if the map is actually used with helpers (e.g.,
> bpf_map_lookup_elem(&map, ...)) it would be invalid for compiler to do
> anything crazy with that map reference, because compiler has no
> visibility into what opaque helpers do with that memory. So I don't
> think it can alias multiple maps, for instance. So I think static maps
> should be fine.

Yeah. That makes sense.

> See above about passing a pointer to map into black box functions. I'd
> bet that the compiler can't merge together two different references at
> least because of that.
> 
> For static maps, btw, just like for static functions and vars, there
> is no symbol, it's an offset into .maps section. We use that offset to
> identify the map itself.

Ok. Sounds like there is a desire to expose both static and static volatile
into skeleton.
Sure, but let's make it such the linking step doesn't change the skeleton.
Imagine a project that using single .bpf.c file and skeleton.
It grows and wants to split itself into multiple .bpf.c.
If such split would change the skeleton generated var/map names
it would be annoying user experience.

I see few options to avoid that:
- keeping the btf names as-is during linking
The final .o can have multiple vars and maps with the same name.
The skeleton gen can see the name collision and disambiguate them.
Here I think it's important to give users a choice. Blindly appending
file name is not ideal.
How to express it cleanly in .bpf.c? I don't know. SEC() would be a bit
ugly. May be similar to core flavors? ___1 and ___2 ? Also not ideal.
- another option is to fail skeleton gen if names conflict.
This way the users wold be able to link just fine and traditonal C style
linker behavior will be preserved, but if the user wants a skeleton
then the static map names across .bpf.c files shouldn't conflict.
imo that's reasonable restriction.
- maybe adopt __hidden for vars and maps? Only not hidden (which is default now)
would be seen in skeleton?