netdev - Re: [RFC PATCH bpf-next 4/8] libbpf: identify maps by section index in addition to offset

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: <4B62AA63-EA75-4C8B-B225-47930BC0812A@fb.com>
Date:   Mon, 17 Jun 2019 18:15:30 +0000
From:   Song Liu <songliubraving@...com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
CC:     Song Liu <liu.song.a23@...il.com>,
        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>
Subject: Re: [RFC PATCH bpf-next 4/8] libbpf: identify maps by section index
 in addition to offset



> On Jun 17, 2019, at 11:06 AM, Andrii Nakryiko <andrii.nakryiko@...il.com> wrote:
> 
> On Sat, Jun 15, 2019 at 2:08 PM Song Liu <liu.song.a23@...il.com> wrote:
>> 
>> On Mon, Jun 10, 2019 at 9:37 PM Andrii Nakryiko <andriin@...com> wrote:
>>> 
>>> To support maps to be defined in multiple sections, it's important to
>>> identify map not just by offset within its section, but section index as
>>> well. This patch adds tracking of section index.
>>> 
>>> For global data, we record section index of corresponding
>>> .data/.bss/.rodata ELF section for uniformity, and thus don't need
>>> a special value of offset for those maps.
>>> 
>>> Signed-off-by: Andrii Nakryiko <andriin@...com>
>>> ---
>>> tools/lib/bpf/libbpf.c | 42 ++++++++++++++++++++++++++----------------
>>> 1 file changed, 26 insertions(+), 16 deletions(-)
>>> 
> 
> <snip>
> 
>>> @@ -3472,13 +3488,7 @@ bpf_object__find_map_fd_by_name(struct bpf_object *obj, const char *name)
>>> struct bpf_map *
>>> bpf_object__find_map_by_offset(struct bpf_object *obj, size_t offset)
>>> {
>>> -       int i;
>>> -
>>> -       for (i = 0; i < obj->nr_maps; i++) {
>>> -               if (obj->maps[i].offset == offset)
>>> -                       return &obj->maps[i];
>>> -       }
>>> -       return ERR_PTR(-ENOENT);
>>> +       return ERR_PTR(-ENOTSUP);
>> 
>> I probably missed some discussion. But is it OK to stop supporting
>> this function?
> 
> This function was added long time ago for some perf (the tool)
> specific use case. But I haven't found any uses of that in kernel
> code, as well as anywhere on github/internal FB code base. It appears
> it's not used anywhere.
> 
> Also, this function makes bad assumption that map can be identified by
> single offset, while we are going to support maps in two (or more, if
> necessary) different ELF sections, so offset is not unique anymore.
> It's not clear what's the intended use case for this API is, looking
> up by name should be the way to do this. Given it's not used, but we
> still need to preserve ABI, I switched it to return -ENOTSUP.

Good survey of the use cases! Yeah, I agree returning -ENOTSUP is the 
best option here. 

Acked-by: Song Liu <songliubraving@...com>