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: <588c763f-1383-d92b-116a-c6826ffa1418@fb.com>
Date:   Fri, 23 Apr 2021 16:26:27 -0700
From:   Yonghong Song <yhs@...com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
CC:     Andrii Nakryiko <andrii.nakryiko@...il.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 4/23/21 4:05 PM, Alexei Starovoitov wrote:
> On Fri, Apr 23, 2021 at 2:56 PM Yonghong Song <yhs@...com> wrote:
>>>>>
>>>>> -static volatile const __u32 print_len;
>>>>> -static volatile const __u32 ret1;
>>>>> +volatile const __u32 print_len = 0;
>>>>> +volatile const __u32 ret1 = 0;
>>>>
>>>> I am little bit puzzled why bpf_iter_test_kern4.c is impacted. I think
>>>> this is not in a static link test, right? The same for a few tests below.
>>>
>>> All the selftests are passed through a static linker, so it will
>>> append obj_name to each static variable. So I just minimized use of
>>> static variables to avoid too much code churn. If this variable was
>>> static, it would have to be accessed as
>>> skel->rodata->bpf_iter_test_kern4__print_len, for example.
>>
>> Okay this should be fine. selftests/bpf specific. I just feel that
>> some people may get confused if they write/see a single program in
>> selftest and they have to use obj_varname format and thinking this
>> is a new standard, but actually it is due to static linking buried
>> in Makefile. Maybe add a note in selftests/README.rst so we
>> can point to people if there is confusion.
> 
> I'm not sure I understand.
> Are you saying that
> bpftool gen object out_file.o in_file.o
> is no longer equivalent to llvm-strip ?

This is more about BTF and ELF.
Give a simple example,
$ cat t1.c
volatile static int aa = 10;
int foo() { return aa; }
$ clang -O2 -g -c -target bpf t1.c

Using bpftool compiled with this patch:
$ bpftool gen object output.o t1.o
$ llvm-readelf -s t1.o | grep aa
      3: 0000000000000000     4 OBJECT  LOCAL  DEFAULT     4 aa
$ llvm-readelf -s output.o | grep aa
      3: 0000000000000000     4 OBJECT  LOCAL  DEFAULT     4 aa

$ bpftool btf dump file t1.o | grep aa
[5] VAR 'aa' type_id=4, linkage=static
$ bpftool btf dump file output.o | grep aa
[5] VAR 't1..aa' type_id=4, linkage=static

So yes you are right, this will affect skeleton user
if you use static linker with single file.

> Since during that step static vars will get their names mangled?
> So a good chunk of code that uses skeleton right now should either
> 1. don't do the linking step
> or
> 2. adjust their code to use global vars
> or
> 3. adjust the usage of skel.h in their corresponding user code
>    to accommodate mangled static names?
> Did it get it right?
>