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

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Sun Jul 26, 2026 at 3:51 AM CEST, sashiko-bot wrote:
> 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?
>

Will be followed up on separately for each arch.

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