netdev - Re: [Potential Spoof] [PATCH bpf-next 06/15] libbpf: expose BPF program's function name

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:   Wed, 11 Dec 2019 11:54:23 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Martin Lau <kafai@...com>
Cc:     Andrii Nakryiko <andriin@...com>,
        "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        "daniel@...earbox.net" <daniel@...earbox.net>,
        Kernel Team <Kernel-team@...com>
Subject: Re: [Potential Spoof] [PATCH bpf-next 06/15] libbpf: expose BPF
 program's function name

On Wed, Dec 11, 2019 at 11:38 AM Martin Lau <kafai@...com> wrote:
>
> On Mon, Dec 09, 2019 at 05:14:29PM -0800, Andrii Nakryiko wrote:
> > Add APIs to get BPF program function name, as opposed to bpf_program__title(),
> > which returns BPF program function's section name. Function name has a benefit
> > of being a valid C identifier and uniquely identifies a specific BPF program,
> > while section name can be duplicated across multiple independent BPF programs.
> >
> > Add also bpf_object__find_program_by_name(), similar to
> > bpf_object__find_program_by_title(), to facilitate looking up BPF programs by
> > their C function names.
> >
> > Convert one of selftests to new API for look up.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> > ---
> >  tools/lib/bpf/libbpf.c                        | 28 +++++++++++++++----
> >  tools/lib/bpf/libbpf.h                        |  9 ++++--
> >  tools/lib/bpf/libbpf.map                      |  2 ++
> >  .../selftests/bpf/prog_tests/rdonly_maps.c    | 11 +++-----
> >  4 files changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> > index edfe1cf1e940..f13752c4d271 100644
> > --- a/tools/lib/bpf/libbpf.c
> > +++ b/tools/lib/bpf/libbpf.c
> > @@ -209,8 +209,8 @@ static const char * const libbpf_type_to_btf_name[] = {
> >  };
> >
> >  struct bpf_map {
> > -     int fd;
> >       char *name;
> > +     int fd;
> This change, and

Oh, no reason beyond eliminating completely unnecessary 4 byte
padding. This one I didn't think is worth mentioning at all, it's just
an internal struct rearrangement.

>
> >       int sec_idx;
> >       size_t sec_offset;
> >       int map_ifindex;
> > @@ -1384,7 +1384,7 @@ static int bpf_object__init_user_btf_maps(struct bpf_object *obj, bool strict,
> >  }
> >
> >  static int bpf_object__init_maps(struct bpf_object *obj,
> > -                              struct bpf_object_open_opts *opts)
> > +                              const struct bpf_object_open_opts *opts)
> here, and a few other const changes,
>
> are all good changes.  If they are not in a separate patch, it will be useful
> to the reviewer if there is commit messages mentioning there are some
> unrelated cleanup changes.  I have been looking at where it may cause
> compiler warning because of this change, or I missed something?

My bad. I could split it out into a separate patch but didn't bother.
The reason for this change is that those opts were always supposed to
be passed as pointer to const struct and it was just an ommision on my
side to not declare them as such. But not doing it in this patch set
would mean that bpf_object__open_skeleton API would have to stick to
non-const opts as well, and I didn't want to proliferate this blunt.

>
> >  {
> >       const char *pin_root_path = OPTS_GET(opts, pin_root_path, NULL);
> >       bool strict = !OPTS_GET(opts, relaxed_maps, false);
> > @@ -1748,6 +1748,19 @@ bpf_object__find_program_by_title(const struct bpf_object *obj,
> >       return NULL;
> >  }
> >

trimming irrelevant parts ;)

[...]