linux-kernel - Re: [PATCH 05/18] bpf-preload: Generate static variables

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, 3 Apr 2022 17:22:38 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Al Viro <viro@...iv.linux.org.uk>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        KP Singh <kpsingh@...nel.org>, Shuah Khan <shuah@...nel.org>,
        "mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
        "alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        "linux-stm32@...md-mailman.stormreply.com" 
        <linux-stm32@...md-mailman.stormreply.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-security-module@...r.kernel.org" 
        <linux-security-module@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 05/18] bpf-preload: Generate static variables

On Wed, Mar 30, 2022 at 12:44 AM Roberto Sassu <roberto.sassu@...wei.com> wrote:
>
> > From: Andrii Nakryiko [mailto:andrii.nakryiko@...il.com]
> > Sent: Wednesday, March 30, 2022 1:52 AM
> > On Mon, Mar 28, 2022 at 10:52 AM Roberto Sassu
> > <roberto.sassu@...wei.com> wrote:
> > >
> > > The first part of the preload code generation consists in generating the
> > > static variables to be used by the code itself: the links and maps to be
> > > pinned, and the skeleton. Generation of the preload variables and
> > methods
> > > is enabled with the option -P added to 'bpftool gen skeleton'.
> > >
> > > The existing variables maps_link and progs_links in bpf_preload_kern.c
> > have
> > > been renamed respectively to dump_bpf_map_link and
> > dump_bpf_prog_link, to
> > > match the name of the variables in the main structure of the light
> > > skeleton.
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> > > ---
> > >  kernel/bpf/preload/bpf_preload_kern.c         |  35 +-
> > >  kernel/bpf/preload/iterators/Makefile         |   2 +-
> > >  .../bpf/preload/iterators/iterators.lskel.h   | 378 +++++++++---------
> > >  .../bpf/bpftool/Documentation/bpftool-gen.rst |   5 +
> > >  tools/bpf/bpftool/bash-completion/bpftool     |   2 +-
> > >  tools/bpf/bpftool/gen.c                       |  27 ++
> > >  tools/bpf/bpftool/main.c                      |   7 +-
> > >  tools/bpf/bpftool/main.h                      |   1 +
> > >  8 files changed, 254 insertions(+), 203 deletions(-)
> > >
> >
> > [...]
> >
> > > +__attribute__((unused)) static void
> > > +iterators_bpf__assert(struct iterators_bpf *s)
> > > +{
> > > +#ifdef __cplusplus
> > > +#define _Static_assert static_assert
> > > +#endif
> > > +#ifdef __cplusplus
> > > +#undef _Static_assert
> > > +#endif
> > > +}
> > > +
> > > +static struct bpf_link *dump_bpf_map_link;
> > > +static struct bpf_link *dump_bpf_prog_link;
> > > +static struct iterators_bpf *skel;
> >
> > I don't understand what is this and what for? You are making an
> > assumption that light skeleton can be instantiated just once, why? And
> > adding extra bpftool option to light skeleton codegen just to save a
> > bit of typing at the place where light skeleton is actually
> > instantiated and used doesn't seems like a right approach.
>
> True, iterator_bpf is simple. Writing the preloading code
> for it is simple. But, what if you wanted to preload an LSM
> with 10 hooks or more?

I suppose you'd write a straightforward code to do pinning ten times
for ten different programs to ten different paths. But with this you
don't have to establish a random set of conventions that might not
apply in all the situations to anyone that would try to use this
feature.

Worst case, light skeleton can be extended to provide a way to iterate
all programs programmatically.

>
> Ok, regarding where the preloading code should be, I will
> try to move the generated code to the kernel module instead
> of the light skeleton.
>
> Thanks
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
> > Further, even if this is the way to go, please split out bpftool
> > changes from kernel changes. There is nothing requiring them to be
> > coupled together.
> >
> > [...]