netdev - Re: [RFC bpf-next 1/3] bpf: add module support to btf display helpers

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: <CAADnVQK6PFAHQMBgQ=Xp7tUFkUBg5yUgBM+r5mi-Kd5UWNWHzw@mail.gmail.com>
Date:   Sat, 14 Nov 2020 08:04:42 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alan Maguire <alan.maguire@...cle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, Martin Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        john fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...omium.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Ingo Molnar <mingo@...hat.com>, Hao Luo <haoluo@...gle.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Quentin Monnet <quentin@...valent.com>,
        open list <linux-kernel@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [RFC bpf-next 1/3] bpf: add module support to btf display helpers

On Fri, Nov 13, 2020 at 10:59 PM Andrii Nakryiko
<andrii.nakryiko@...il.com> wrote:
>
> On Fri, Nov 13, 2020 at 10:11 AM Alan Maguire <alan.maguire@...cle.com> wrote:
> >
> > bpf_snprintf_btf and bpf_seq_printf_btf use a "struct btf_ptr *"
> > argument that specifies type information about the type to
> > be displayed.  Augment this information to include a module
> > name, allowing such display to support module types.
> >
> > Signed-off-by: Alan Maguire <alan.maguire@...cle.com>
> > ---
> >  include/linux/btf.h            |  8 ++++++++
> >  include/uapi/linux/bpf.h       |  5 ++++-
> >  kernel/bpf/btf.c               | 18 ++++++++++++++++++
> >  kernel/trace/bpf_trace.c       | 42 ++++++++++++++++++++++++++++++++----------
> >  tools/include/uapi/linux/bpf.h |  5 ++++-
> >  5 files changed, 66 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/linux/btf.h b/include/linux/btf.h
> > index 2bf6418..d55ca00 100644
> > --- a/include/linux/btf.h
> > +++ b/include/linux/btf.h
> > @@ -209,6 +209,14 @@ static inline const struct btf_var_secinfo *btf_type_var_secinfo(
> >  const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
> >  const char *btf_name_by_offset(const struct btf *btf, u32 offset);
> >  struct btf *btf_parse_vmlinux(void);
> > +#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> > +struct btf *bpf_get_btf_module(const char *name);
> > +#else
> > +static inline struct btf *bpf_get_btf_module(const char *name)
> > +{
> > +       return ERR_PTR(-ENOTSUPP);
> > +}
> > +#endif
> >  struct btf *bpf_prog_get_target_btf(const struct bpf_prog *prog);
> >  #else
> >  static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 162999b..26978be 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -3636,7 +3636,8 @@ struct bpf_stack_build_id {
> >   *             the pointer data is carried out to avoid kernel crashes during
> >   *             operation.  Smaller types can use string space on the stack;
> >   *             larger programs can use map data to store the string
> > - *             representation.
> > + *             representation.  Module-specific data structures can be
> > + *             displayed if the module name is supplied.
> >   *
> >   *             The string can be subsequently shared with userspace via
> >   *             bpf_perf_event_output() or ring buffer interfaces.
> > @@ -5076,11 +5077,13 @@ struct bpf_sk_lookup {
> >   * potentially to specify additional details about the BTF pointer
> >   * (rather than its mode of display) - is included for future use.
> >   * Display flags - BTF_F_* - are passed to bpf_snprintf_btf separately.
> > + * A module name can be specified for module-specific data.
> >   */
> >  struct btf_ptr {
> >         void *ptr;
> >         __u32 type_id;
> >         __u32 flags;            /* BTF ptr flags; unused at present. */
> > +       const char *module;     /* optional module name. */
>
> I think module name is a wrong API here, similarly how type name was
> wrong API for specifying the type (and thus we use type_id here).
> Using the module's BTF ID seems like a more suitable interface. That's
> what I'm going to use for all kinds of existing BPF APIs that expect
> BTF type to attach BPF programs.
>
> Right now, we use only type_id and implicitly know that it's in
> vmlinux BTF. With module BTFs, we now need a pair of BTF object ID +
> BTF type ID to uniquely identify the type. vmlinux BTF now can be
> specified in two different ways: either leaving BTF object ID as zero
> (for simplicity and backwards compatibility) or specifying it's actual
> BTF obj ID (which pretty much always should be 1, btw). This feels
> like a natural extension, WDYT?
>
> And similar to type_id, no one should expect users to specify these
> IDs by hand, Clang built-in and libbpf should work together to figure
> this out for the kernel to use.
>
> BTW, with module names there is an extra problem for end users. Some
> types could be either built-in or built as a module (e.g., XFS data
> structures). Why would we require BPF users to care which is the case
> on any given host?

+1.
As much as possible libbpf should try to hide the difference between
type in a module vs type in the vmlinux, since that difference most of the
time is irrelevant from bpf prog pov.