Re: [PATCH bpf-next 2/5] bpf: Keep the BPF calling convention for a trampoline on a BPF function

[Yonghong Song <yonghong.song@linux.dev>]

On 10/10/26 5:00 PM, Alexei Starovoitov wrote:
> On Thu, Oct 08, 2026 at 11:42 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>> Mark the model of a target program's function as bpf_tgt_prog, and have
>> bpf_jit_place_args() leave every slot of such a model where the BPF
>> convention put it. That is where both JITs pass it in a call from one
>> BPF function to another: slot N where the kernel convention passes its
>> Nth eightbyte, in a register or a stack slot. The x86-64 trampoline does
>> not use bpf_jit_place_args() yet; the next patch makes it, and this
>> keeps it right for a BPF target.
> can we drop the flag and for tgt_prog rewrite the model in
> bpf_check_attach_target() as one u64 arg per argument slot ?
> Every convention places N u64 args in slot order,
> so no arch has to know about BPF targets.

Good point. Will do this with u64 arg per argument slot.

>
> The flag fixes bpf_jit_place_args() only.
> calc_arg_aux() on arm64 picks reg vs stack per argument by looking
> at its first slot. In callee_stack() from patch 5 'h' is in slots 7 and 8.
> pos_of_slot[7] is x7, so the whole 'h' is counted in args_in_regs and
> save_args() emits 'str x8' for the second half. x8 is BPF R0.
> ctx[8] is garbage. For fexit it does 'ldr x8' and the first outgoing
> stack slot is never written, so callee_stack() runs with garbage
> in the upper half of 'h'. The test should fail on arm64.

Indeed, this is an issue. I should test with llvm23 for arm64 as well.

> CI uses clang 21 without stack args and won't catch it.
>
> s390 trampoline doesn't use bpf_jit_place_args().
> s390 ABI passes 16 byte arg by reference, so it does:
>    mvc bpf_arg_off(16,%r15),0(%arg)
> For callee_odd() that is a load from address 8.
> It also saves and restores nr_args == 3 registers,
> while callee_odd() takes 4.
>
> With the flat model arm64, s390 and the r9/stack split on x86
> are fixed without touching arch code.
>
>> @@ -21620,6 +21620,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>>   		ret = btf_distill_func_proto(log, btf, t, tname, &tgt_info->fmodel);
>>   		if (ret < 0)
>>   			return ret;
>> +		tgt_info->fmodel.bpf_tgt_prog = !!tgt_prog;
> There is one more issue with BPF targets. Few lines above:
> 	if (tgt_prog && conservative)
> 		t = NULL;
> and the model is 5 u64.
> A static function with 6+ args where one of them is a plain pointer
> is 'unreliable', since btf_prepare_func_args() returns -EINVAL,
> but it still takes stack args.
> The trampoline saves and restores 5 registers. After fentry prog
> r9 on x86 and x5-x7 on arm64 have whatever __bpf_prog_exit() left there.
> With fexit the stack slots are not copied either.
> The function continues with garbage in the args that the verifier
> checked as pointers, I think.
> Reject such attach ?

I agree that rejection is the right way to go.

>
> Pls tell your agent to test on arm64 too.

Yes, will do.

>
> pw-bot: cr