linux-kernel - Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from print_bpf

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, 21 Mar 2018 19:37:49 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Quentin Monnet <quentin.monnet@...ronome.com>
Cc:     Jiri Olsa <jolsa@...nel.org>, Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        lkml <linux-kernel@...r.kernel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH 1/2] bpf: Remove struct bpf_verifier_env argument from
 print_bpf_insn

On Wed, Mar 21, 2018 at 05:25:33PM +0000, Quentin Monnet wrote:
> 2018-03-21 16:02 UTC+0100 ~ Jiri Olsa <jolsa@...nel.org>
> > We use print_bpf_insn in user space (bpftool and soon perf),
> > so it'd be nice to keep it generic and strip it off the kernel
> > struct bpf_verifier_env argument.
> > 
> > This argument can be safely removed, because its users can
> > use the struct bpf_insn_cbs::private_data to pass it.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> >  kernel/bpf/disasm.c   | 52 +++++++++++++++++++++++++--------------------------
> >  kernel/bpf/disasm.h   |  5 +----
> >  kernel/bpf/verifier.c |  6 +++---
> >  3 files changed, 30 insertions(+), 33 deletions(-)
> > 
> 
> [...]
> 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index c6eff108aa99..9f27d3fa7259 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -202,8 +202,7 @@ EXPORT_SYMBOL_GPL(bpf_verifier_log_write);
> >   * generic for symbol export. The function was renamed, but not the calls in
> >   * the verifier to avoid complicating backports. Hence the alias below.
> >   */
> > -static __printf(2, 3) void verbose(struct bpf_verifier_env *env,
> > -				   const char *fmt, ...)
> > +static __printf(2, 3) void verbose(void *private_data, const char *fmt, ...)
> >  	__attribute__((alias("bpf_verifier_log_write")));
> 
> Just as a note, verbose() will be aliased to a function whose prototype
> differs (bpf_verifier_log_write() still expects a struct
> bpf_verifier_env as its first argument). I am not so familiar with
> function aliases, could this change be a concern?

yea, but as it was pointer for pointer switch I did not
see any problem with that.. I'll check more

jirka