netdev - Re: [PATCH v2] Add BPF_SYNCHRONIZE

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:   Sun, 29 Jul 2018 13:46:44 -0700
From:   Daniel Colascione <dancol@...gle.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Joel Fernandes <joelaf@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Tim Murray <timmurray@...gle.com>,
        Network Development <netdev@...r.kernel.org>,
        Lorenzo Colitti <lorenzo@...gle.com>,
        Chenbo Feng <fengc@...gle.com>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH v2] Add BPF_SYNCHRONIZE_MAPS bpf(2) command

On Sun, Jul 29, 2018 at 8:51 AM, Alexei Starovoitov <
alexei.starovoitov@...il.com> wrote:

> On Thu, Jul 26, 2018 at 7:51 PM, Daniel Colascione <dancol@...gle.com>
> wrote:
> > BPF_SYNCHRONIZE_MAPS waits for the release of any references to a BPF
> > map made by a BPF program that is running at the time the
> > BPF_SYNCHRONIZE_MAPS command is issued. The purpose of this command is
> > to provide a means for userspace to replace a BPF map with another,
> > newer version, then ensure that no component is still using the "old"
> > map before manipulating the "old" map in some way.
> >
> > Signed-off-by: Daniel Colascione <dancol@...gle.com>
> > ---
> >  include/uapi/linux/bpf.h |  9 +++++++++
> >  kernel/bpf/syscall.c     | 13 +++++++++++++
> >  2 files changed, 22 insertions(+)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index b7db3261c62d..5b27e9117d3e 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -75,6 +75,14 @@ struct bpf_lpm_trie_key {
> >         __u8    data[0];        /* Arbitrary size */
> >  };
> >
> > +/* BPF_SYNCHRONIZE_MAPS waits for the release of any references to a
> > + * BPF map made by a BPF program that is running at the time the
> > + * BPF_SYNCHRONIZE_MAPS command is issued. The purpose of this command
>
> that doesn't sound right to me.
> such command won't wait for the release of the references.
> in case of map-in-map the program does not hold
> the references to inner map (only to outer map).
>

Well, today that's the guarantee it provides. :-) I figured it'd be simpler
to explain the guarantee this way.

How about "waits for the release of any reference to a map obtained from
another map"?


>
> > + * is to provide a means for userspace to replace a BPF map with
> > + * another, newer version, then ensure that no component is still
> > + * using the "old" map before manipulating the "old" map in some way.
> > + */
>
> that's correct, but the whole paragraph still reads too
> 'generic' which will lead to confusion,
> whereas the use case is map-in-map only.
> I think bpf_sync_inner_map or
> bpf_sync_map_in_map would be better
> choices for the name.


I worry that a name like that would be too specific.

Content of type "text/html" skipped