linux-kernel - Re: [PATCH v2 2/2] x86/cfi,bpf: Fix BPF JIT call

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: <20231206163814.GB36423@noisy.programming.kicks-ass.net>
Date:   Wed, 6 Dec 2023 17:38:14 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Jiri Olsa <olsajiri@...il.com>, Song Liu <song@...nel.org>,
        Song Liu <songliubraving@...a.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        X86 ML <x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
        "David S. Miller" <davem@...emloft.net>,
        David Ahern <dsahern@...nel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <martin.lau@...ux.dev>,
        Yonghong Song <yonghong.song@...ux.dev>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Stanislav Fomichev <sdf@...gle.com>,
        Hao Luo <haoluo@...gle.com>, Arnd Bergmann <arnd@...db.de>,
        Sami Tolvanen <samitolvanen@...gle.com>,
        Kees Cook <keescook@...omium.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        linux-riscv <linux-riscv@...ts.infradead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Network Development <netdev@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        clang-built-linux <llvm@...ts.linux.dev>,
        Josh Poimboeuf <jpoimboe@...nel.org>,
        Joao Moreira <joao@...rdrivepizza.com>,
        Mark Rutland <mark.rutland@....com>
Subject: Re: [PATCH v2 2/2] x86/cfi,bpf: Fix BPF JIT call

On Mon, Dec 04, 2023 at 05:18:31PM -0800, Alexei Starovoitov wrote:

> [   13.978497]  ? asm_exc_invalid_op+0x1a/0x20
> [   13.978798]  ? tcp_set_ca_state+0x51/0xd0
> [   13.979087]  tcp_v6_syn_recv_sock+0x45c/0x6c0
> [   13.979401]  tcp_check_req+0x497/0x590

> The stack trace doesn't have any bpf, but it's a bpf issue too.
> Here tcp_set_ca_state() calls
> icsk->icsk_ca_ops->set_state(sk, ca_state);
> which calls bpf prog via bpf trampoline.



Specifically, I think this is
tools/testing/selftests/bpf/progs/bpf_cubic.c, which has:

        .set_state      = (void *)bpf_cubic_state,

which comes from:

BPF_STRUCT_OPS(bpf_cubic_state, struct sock *sk, __u8 *new_state)

which then wraps:

BPF_PROG()

which ends up generating:

static __always_inline ___bpf_cubic_state(unsigned long long *ctx, struct sock *sk, __u8 *new_state)
{
	...
}

void bpf_cubic_state(unsigned long long *ctx)
{
	return ____bpf_cubic_state(ctx, ctx[0], ctx[1]);
}


I think this then uses arch_prepare_bpf_trampoline(), but I'm entirely
lost how this all comes together, because the way I understand it the
whole bpf_trampoline is used to hook into an ftrace __fentry hook.

And a __fentry hook is very much not a function pointer. Help!?!?


The other case:

For tools/testing/selftests/bpf/progs/bloom_filter_bench.c we have:

        bpf_for_each_map_elem(&array_map, bloom_callback, &data, 0);

and here bloom callback appears like a normal function:

static __u64
bloom_callback(struct bpf_map *map, __u32 *key, void *val,
               struct callback_ctx *data)


But what do functions looks like in the JIT? What's the actual address
that's then passed into the helper function. Given this seems to work
without kCFI, it should at least have an ENDBR, but there's only 3 of
those afaict:

  - emit_prologue() first insn
  - emit_prologue() tail-call site
  - arch_preprare_bpf_trampoline()

If the function passed to the helper is from do_jit()/emit_prologue(),
then how do I tell what 'function' is being JIT'ed ?

If it is arch_prepare_bpf_trampoline(), then we're back at the previous
question and I don't see how a __fentry site becomes a callable function
pointer.


Any clues would be much appreciated.