netdev - Re: [PATCH bpf-next] selftests/bpf: add BPF_HANDLER, BPF_KPROBE, and 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] [day] [month] [year] [list]

Message-ID: <CAEf4BzZsZ1hs-DG5WJ144hRoqFD_yLeKghV-FkboJ6B1_UJQxA@mail.gmail.com>
Date:   Thu, 9 Jan 2020 14:35:39 -0800
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Andrii Nakryiko <andriin@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Yonghong Song <yhs@...com>, Martin Lau <kafai@...com>,
        Kernel Team <kernel-team@...com>
Subject: Re: [PATCH bpf-next] selftests/bpf: add BPF_HANDLER, BPF_KPROBE, and
 BPF_KRETPROBE macros

On Thu, Jan 9, 2020 at 10:48 AM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Dec 26, 2019 at 01:18:55PM -0800, Andrii Nakryiko wrote:
> > Streamline BPF_TRACE_x macro by moving out return type and section attribute
> > definition out of macro itself. That makes those function look in source code
> > similar to other BPF programs. Additionally, simplify its usage by determining
> > number of arguments automatically (so just single BPF_TRACE vs a family of
> > BPF_TRACE_1, BPF_TRACE_2, etc). Also, allow more natural function argument
> > syntax without commas inbetween argument type and name.
> >
> > Given this helper is useful not only for tracing tp_btf/fenty/fexit programs,
> > but could be used for LSM programs and others following the same pattern,
> > rename BPF_TRACE macro into more generic BPF_HANDLER. Existing BPF_TRACE_x
> > usages in selftests are converted to new BPF_HANDLER macro.
> >
> > Following the same pattern, define BPF_KPROBE and BPF_KRETPROBE macros for
> > nicer usage of kprobe/kretprobe arguments, respectively. BPF_KRETPROBE, adopts
> > same convention used by fexit programs, that last defined argument is probed
> > function's return result.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@...com>
> ...
> > +
> > +#define BPF_HANDLER(name, args...)                                       \
> > +name(unsigned long long *ctx);                                                   \
> > +static __always_inline typeof(name(0)) ____##name(args);                 \
> > +typeof(name(0)) name(unsigned long long *ctx)                                    \
> > +{                                                                        \
> > +     _Pragma("GCC diagnostic push")                                      \
> > +     _Pragma("GCC diagnostic ignored \"-Wint-conversion\"")              \
> > +     return ____##name(___bpf_ctx_cast(args));                           \
> > +     _Pragma("GCC diagnostic pop")                                       \
>
> It says "GCC ..", but does it actually work with gcc?
> If the answer is no, I think it's still ok, but would be good to document.

I'll write a simple test program to verify. I expect it to work, of course :)

>
> Other than the above BPF_HANDLER, BPF_KPROBE, BPF_KRETPROBE distinction make sense.
> Please document it. It's not obvious when to use each one.

Yep, will do.

>
> Also the intent is do let users do s/BPF_KRETPROBE/BPF_HANDLER/ conversion
> when they transition from kretprobe to fexit without changing anything else
> in the function body and function declaration? That's neat if that can work.

Yep, it should be a trivial s/BPF_KPROBE/BPF_HANDLER/ change.