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

Message-ID: <CAEf4BzYKtA9Hk5oswZVD_pZ-VxjXXd_OV_bRs+42cfgf8dqodw@mail.gmail.com>
Date:   Mon, 17 Jun 2019 11:06:21 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Song Liu <liu.song.a23@...il.com>
Cc:     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 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.

>
> Thanks,
> Song
>
> >  }
> >
> >  long libbpf_get_error(const void *ptr)
> > --
> > 2.17.1
> >