netdev - Re: [PATCH bpf-next v3 2/4] bpf, netns: Keep attached programs in bpf_prog

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]

Date:   Fri, 26 Jun 2020 15:15:40 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Jakub Sitnicki <jakub@...udflare.com>
Cc:     bpf <bpf@...r.kernel.org>, Networking <netdev@...r.kernel.org>,
        kernel-team <kernel-team@...udflare.com>
Subject: Re: [PATCH bpf-next v3 2/4] bpf, netns: Keep attached programs in bpf_prog_array

On Fri, Jun 26, 2020 at 3:13 PM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> On Fri, Jun 26, 2020 at 2:45 AM Jakub Sitnicki <jakub@...udflare.com> wrote:
> >
> > On Thu, Jun 25, 2020 at 10:50 PM CEST, Andrii Nakryiko wrote:
> > > On Thu, Jun 25, 2020 at 7:17 AM Jakub Sitnicki <jakub@...udflare.com> wrote:
> > >>
> > >> Prepare for having multi-prog attachments for new netns attach types by
> > >> storing programs to run in a bpf_prog_array, which is well suited for
> > >> iterating over programs and running them in sequence.
> > >>
> > >> After this change bpf(PROG_QUERY) may block to allocate memory in
> > >> bpf_prog_array_copy_to_user() for collected program IDs. This forces a
> > >> change in how we protect access to the attached program in the query
> > >> callback. Because bpf_prog_array_copy_to_user() can sleep, we switch from
> > >> an RCU read lock to holding a mutex that serializes updaters.
> > >>
> > >> Because we allow only one BPF flow_dissector program to be attached to
> > >> netns at all times, the bpf_prog_array pointed by net->bpf.run_array is
> > >> always either detached (null) or one element long.
> > >>
> > >> No functional changes intended.
> > >>
> > >> Signed-off-by: Jakub Sitnicki <jakub@...udflare.com>
> > >> ---
> > >
> > > I wonder if instead of NULL prog_array, it's better to just use a
> > > dummy empty (but allocated) array. Might help eliminate some of the
> > > IFs, maybe even in the hot path.
> >
> > That was my initial approach, which I abandoned seeing that it leads to
> > replacing NULL prog_array checks in flow_dissector with
> > bpf_prog_array_is_empty() checks to determine which netns has a BPF
> > program attached. So no IFs gone there.
> >
> > While on the hot path, where we run the program, we probably would still
> > be left with an IF checking for empty prog_array to avoid building the
> > context if no progs will RUN.
> >
> > The checks I'm referring to are on attach path, in
> > flow_dissector_bpf_prog_attach_check(), and hot-path,
> > __skb_flow_dissect().
> >
>
> Fair enough.
>

Acked-by: Andrii Nakryiko <andriin@...com>