From: "Kumar Kartikeya Dwivedi" <memxor@gmail.com>
To: <sashiko-reviews@lists.linux.dev>,
"Kumar Kartikeya Dwivedi" <memxor@gmail.com>
Cc: <bpf@vger.kernel.org>
Subject: Re: [PATCH bpf-next v1 8/9] bpf, x86: Fix stack-passed arguments for indirect trampolines
Date: Thu, 16 Jul 2026 13:32:41 +0200 [thread overview]
Message-ID: <DJZYO1XU6ANR.27W4M62WISKOV@gmail.com> (raw)
In-Reply-To: <20260715221412.ACF3D1F000E9@smtp.kernel.org>
On Thu Jul 16, 2026 at 12:14 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Medium] The patch correctly fixes the struct_ops stack argument offset bug for x86, but leaves the exact same bug unfixed in the ARM64, RISC-V, and LoongArch BPF JIT implementations.
> --
>
> --- Patch [8]: [PATCH bpf-next v1 8/9] bpf, x86: Fix stack-passed arguments for indirect trampolines ---
> commit 804aec167442bbd8ddbbc13495bc188618bbaeaf
> Author: Tejun Heo <tj@kernel.org>
>
> bpf, x86: Fix stack-passed arguments for indirect trampolines
>
> This commit fixes a bug in the x86 BPF JIT where stack-passed arguments
> for indirect trampolines (such as struct_ops) were read from the wrong
> offset. It correctly adjusts the offset to account for only the caller's
> return address being on the stack, which fixes callbacks that have more
> than six arguments.
>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 2f25353f135bd..672fe54dfeb8d 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -3074,6 +3074,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: Medium]
> This is a pre-existing issue, but does this same bug also need to be fixed
> in the ARM64, RISC-V, and LoongArch BPF JIT implementations?
>
> On ARM64, save_args() unconditionally hardcodes a 32-byte offset:
>
> arch/arm64/net/bpf_jit_comp.c:save_args() {
> ...
> soff = 32; /* on stack arguments start from FP + 32 */
> ...
> }
>
Yes, but it will be sent separately as a separate patch.
> This seems to assume two 16-byte frames (FP+LR and FP+R9), but struct_ops
> only pushes one, meaning stack arguments would start at FP + 16.
>
> Similarly, on RISC-V and LoongArch, store_args() unconditionally skips 16
> bytes for the traced function's T0 and FP frame:
>
> arch/riscv/net/bpf_jit_comp64.c:store_args() {
> ...
> /* skip slots for T0 and FP of traced function */
> emit_ld(RV_REG_T1, 16 + (i - RV_MAX_REG_ARGS) * 8, RV_REG_FP, ctx);
> ...
> }
>
> For BPF_TRAMP_F_INDIRECT (is_struct_ops), the prologue aligns FP directly
> to the caller's SP, so arguments start exactly at FP. Will these hardcoded
> offsets cause the JIT to read 16 bytes past the actual arguments on these
> architectures as well?
>
>> int i, j, slot = 0;
>>
>> /* Store function arguments to stack.
next prev parent reply other threads:[~2026-07-16 11:32 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-15 22:00 [PATCH bpf-next v1 0/9] Add arena argument support to kfuncs and struct_ops Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 1/9] bpf: Support __arena and __arena_nullable kfunc argument suffixes Kumar Kartikeya Dwivedi
2026-07-15 23:05 ` bot+bpf-ci
2026-07-16 11:31 ` Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 2/9] bpf: Support __arena and __arena_nullable on struct_ops stub arguments Kumar Kartikeya Dwivedi
2026-07-15 22:22 ` sashiko-bot
2026-07-15 22:00 ` [PATCH bpf-next v1 3/9] bpf, x86: JIT __arena kfunc argument rebasing Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 4/9] bpf, x86: Convert struct_ops arena arguments in the trampoline Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 5/9] selftests/bpf: Add kfunc __arena and __arena_nullable argument tests Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 6/9] selftests/bpf: Add JIT-sequence tests for __arena kfunc arguments Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 7/9] selftests/bpf: Add struct_ops __arena and __arena_nullable argument tests Kumar Kartikeya Dwivedi
2026-07-15 22:00 ` [PATCH bpf-next v1 8/9] bpf, x86: Fix stack-passed arguments for indirect trampolines Kumar Kartikeya Dwivedi
2026-07-15 22:14 ` sashiko-bot
2026-07-16 11:32 ` Kumar Kartikeya Dwivedi [this message]
2026-07-15 22:51 ` bot+bpf-ci
2026-07-15 22:00 ` [PATCH bpf-next v1 9/9] selftests/bpf: Test stack-passed struct_ops arena arguments Kumar Kartikeya Dwivedi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DJZYO1XU6ANR.27W4M62WISKOV@gmail.com \
--to=memxor@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.