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

"Kumar Kartikeya Dwivedi" <[email protected]>
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf
Message-ID <[email protected]>
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 <[email protected]>
>
> 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.
> [ ... ]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.