Re: [PATCH bpf-next v4 11/13] bpf, x86: Fix stack-passed arguments for indirect trampolines

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for stack-passed arguments in struct_ops trampolines leaves identical bugs unfixed on ARM64, RISC-V, and LoongArch
--

--- Patch [11]: [PATCH bpf-next v4 11/13] bpf, x86: Fix stack-passed arguments for indirect trampolines ---
commit 08733054ec575a092fdc6b844fb5b2af4d47355a
Author: Tejun Heo <[email protected]>

bpf, x86: Fix stack-passed arguments for indirect trampolines

This commit corrects the stack offset for reading stack-passed arguments
in indirect trampolines on x86. It adjusts the offset from rbp + 24 to
rbp + 16 to account for the single return address on the stack when the
trampoline is called through a function pointer.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index cb8ee0f3a6423..5c5c32aba2396 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -3075,6 +3075,7 @@ static void save_args(const struct btf_func_model *m, u8 **prog,
>  {
>  	int arg_regs, first_off = 0, nr_regs = 0, nr_stack_slots = 0;
>  	bool use_jmp = bpf_trampoline_use_jmp(flags);
> +	int stack_args_off = (use_jmp || (flags & BPF_TRAMP_F_INDIRECT)) ? 16 : 24;

[Severity: High]
This is a pre-existing issue, but while fixing this for x86, are arm64, 
riscv, and loongarch also affected by the same struct_ops stack-passed 
arguments bug?

Looking at arm64 in arch/arm64/net/bpf_jit_comp.c:save_args(), the offset
seems unconditionally hardcoded to 32, assuming an fentry stack frame layout:

soff = 32; /* on stack arguments start from FP + 32 */

Similarly for riscv in arch/riscv/net/bpf_jit_comp64.c:store_args():

emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);

And loongarch in arch/loongarch/net/bpf_jit.c:store_args():

emit_insn(ctx, ldd, LOONGARCH_GPR_T1, LOONGARCH_GPR_FP,
          16 + (i - LOONGARCH_MAX_REG_ARGS) * 8);

Do these architectures also need a similar adjustment for 
BPF_TRAMP_F_INDIRECT to avoid reading out-of-bounds or garbage values from 
the stack for struct_ops callbacks with more than 8 arguments?

>  	int i, j;
>  
>  	/* Store function arguments to stack.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11