netdev - Re: [PATCH libbpf] libbpf: check if pin

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]

Date:   Fri, 2 Oct 2020 11:13:41 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc:     Hangbin Liu <liuhangbin@...il.com>, bpf <bpf@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>,
        Networking <netdev@...r.kernel.org>,
        Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH libbpf] libbpf: check if pin_path was set even map fd exist

On Fri, Oct 2, 2020 at 4:56 AM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> On Fri, Oct 02, 2020 at 03:57:50PM +0800, Hangbin Liu wrote:
> > Say a user reuse map fd after creating a map manually and set the
> > pin_path, then load the object via libbpf.
> >
> > In libbpf bpf_object__create_maps(), bpf_object__reuse_map() will
> > return 0 if there is no pinned map in map->pin_path. Then after
> > checking if map fd exist, we should also check if pin_path was set
> > and do bpf_map__pin() instead of continue the loop.
> >
> > Fix it by creating map if fd not exist and continue checking pin_path
> > after that.
> >
> > Suggested-by: Andrii Nakryiko <andrii.nakryiko@...il.com>
> > Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
> > ---
> >  tools/lib/bpf/libbpf.c | 75 +++++++++++++++++++++---------------------
> >  1 file changed, 37 insertions(+), 38 deletions(-)
> >

[...]

> > +
> > +                     if (map->init_slots_sz) {
>
> Couldn't we flatten the code by inverting the logic here and using goto?

I explicitly asked Hangbin to not use goto to alter control flow here,
I'd like to keep goto within libbpf mostly for error handling.

>
>         if (!map->init_slot_sz) {
>                 pr_debug("map '%s': skipping creation (preset fd=%d)\n",
>                          map->name, map->fd);
>                 goto map_pin;
>         }
>
>         (...)
> map_pin:
>         if (map->pin_path && !map->pinned) {
>
> If I'm reading this right.
>

[...]

> > --
> > 2.25.4
> >