Re: [PATCH bpf-next v2 8/9] bpf, x86: Fix stack-passed arguments for indirect trampolines

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch fixes the stack-passed argument offset for indirect struct_ops trampolines on x86, but leaves the exact same logical bug unfixed in the ARM64, RISC-V, and LoongArch BPF JIT compilers.
--

--- Patch [8]: [PATCH bpf-next v2 8/9] bpf, x86: Fix stack-passed arguments for indirect trampolines ---
commit 1625042935fc87b38acf83c8304433545d672f35
Author: Tejun Heo <tj@kernel.org>

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

This patch fixes the calculation of stack-passed arguments for indirect
trampolines in the x86 BPF JIT compiler. It introduces stack_args_off
to properly handle the number of return addresses on the stack depending
on how the trampoline was called.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index dfb5335ad8374..4b349ae35ebf9 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 does this same logical bug need to be fixed
for ARM64, RISC-V, and LoongArch? 

Looking at arch/arm64/net/bpf_jit_comp.c:save_args(), there is a hardcoded
offset that unconditionally assumes two frames were pushed:

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

Similarly, in arch/riscv/net/bpf_jit_comp64.c:store_args(), there is a
hardcoded offset of 16 which skips the first 16 bytes of caller arguments for
struct_ops:

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

And arch/loongarch/net/bpf_jit.c:store_args() shares the same hardcoded offset:

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

If a struct_ops callback with more than 8 arguments (or 6 on some
architectures) is executed on these platforms, could it receive out-of-bounds
stack memory instead of the actual arguments?

>  	int i, j, slot = 0;
>  
>  	/* Store function arguments to stack.
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726013105.3689867-1-memxor@gmail.com?part=8