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

Peter Zijlstra
2023-12-06T16:38:35Z

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.