linux-kernel - Re: [PATCH bpf-next v4 2/6] bpf: introduce BPF dispatcher

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:   Mon, 15 Aug 2022 07:31:23 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Björn Töpel <bjorn.topel@...il.com>,
        Network Development <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Björn Töpel <bjorn.topel@...el.com>,
        bpf <bpf@...r.kernel.org>,
        Magnus Karlsson <magnus.karlsson@...il.com>,
        "Karlsson, Magnus" <magnus.karlsson@...el.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Edward Cree <ecree@...arflare.com>,
        Toke Høiland-Jørgensen <thoiland@...hat.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Andrii Nakryiko <andrii.nakryiko@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Christoph Hellwig <hch@...radead.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH bpf-next v4 2/6] bpf: introduce BPF dispatcher

On Mon, Aug 15, 2022 at 7:13 AM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Wed, 11 Dec 2019 13:30:13 +0100
> Björn Töpel <bjorn.topel@...il.com> wrote:
>
> > From: Björn Töpel <bjorn.topel@...el.com>
> >
> > The BPF dispatcher is a multi-way branch code generator, mainly
> > targeted for XDP programs. When an XDP program is executed via the
> > bpf_prog_run_xdp(), it is invoked via an indirect call. The indirect
> > call has a substantial performance impact, when retpolines are
> > enabled. The dispatcher transform indirect calls to direct calls, and
> > therefore avoids the retpoline. The dispatcher is generated using the
> > BPF JIT, and relies on text poking provided by bpf_arch_text_poke().
> >
> > The dispatcher hijacks a trampoline function it via the __fentry__ nop
>
> Why was the ftrace maintainers not Cc'd on this patch?  I would have NACKED
> it. Hell, it wasn't even sent to LKML! This was BPF being sneaky in
> updating major infrastructure of the Linux kernel without letting the
> stakeholders of this change know about it.
>
> For some reason, the BPF folks think they own the entire kernel!
>
> When I heard that ftrace was broken by BPF I thought it was something
> unique they were doing, but unfortunately, I didn't investigate what they
> were doing at the time.

ftrace is still broken and refusing to accept the fact doesn't make it
non-broken.

> Then they started sending me patches to hide fentry locations from ftrace.
> And even telling me that fentry != ftrace

It sounds that you've invented nop5 and kernel's ability
to replace nop5 with a jump or call.
ftrace should really stop trying to own all of the kernel text rewrites.
It's in the way. Like this case.

>    https://lore.kernel.org/all/CAADnVQJTT7h3MniVqdBEU=eLwvJhEKNLSjbUAK4sOrhN=zggCQ@mail.gmail.com/
>
> Even though fentry was created for ftrace
>
>    https://lore.kernel.org/lkml/1258720459.22249.1018.camel@gandalf.stny.rr.com/
>
> and all the work with fentry was for the ftrace infrastructure. Ftrace
> takes a lot of care for security and use cases for other users (like
> live kernel patching). But BPF has the NIH syndrome, and likes to own
> everything and recreate the wheel so that they have full control.
>
> > of the trampoline. One dispatcher instance currently supports up to 64
> > dispatch points. A user creates a dispatcher with its corresponding
> > trampoline with the DEFINE_BPF_DISPATCHER macro.
>
> Anyway, this patch just looks like a re-implementation of static_calls:

It was implemented long before static_calls made it to the kernel
and it's different. Please do your home work.