Re: [PATCH bpf-next v5 13/16] bpf,x86: Implement JIT support for stack arguments

[sashiko-bot@kernel.org]

Sashiko AI review found 4 potential issue(s):
- [High] Data corruption of Argument 6 across function calls due to caller-saved register clobbering.
- [High] Missing size handling for Argument 6 allows verifier bounds tracking bypass and out-of-bounds memory writes.
- [High] The offset translation formula for outgoing arguments 7+ reverses the memory layout, corrupting sub-slot accesses.
- [Medium] The JIT misroutes stack accesses because it rigidly assumes LDX only targets incoming arguments and STX/ST only target outgoing arguments.
--

commit 41dcf99330b6b0a3804185f2a284a76be8340ed1
Author: Yonghong Song <yonghong.song@linux.dev>

bpf,x86: Implement JIT support for stack arguments

Add x86_64 JIT support for BPF functions and kfuncs with more than
5 arguments. The extra arguments are passed through a stack area
addressed by register r11 (BPF_REG_PARAMS) in BPF bytecode,
which the JIT translates to native code.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 32864dbc2c4e8..25b4357de023b 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -1664,16 +1692,45 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
>  	stack_depth = bpf_prog->aux->stack_depth;
> +	stack_arg_depth = bpf_prog->aux->stack_arg_depth;
> +	incoming_stack_arg_depth = bpf_prog->aux->incoming_stack_arg_depth;
> +	outgoing_stack_arg_depth = stack_arg_depth - incoming_stack_arg_depth;
>  	priv_stack_ptr = bpf_prog->aux->priv_stack_ptr;
>  	if (priv_stack_ptr) {
>  		priv_frame_ptr = priv_stack_ptr + PRIV_STACK_GUARD_SZ + round_up(stack_depth, 8);
>  		stack_depth = 0;
>  	}
>  
> +	/*
> +	 * Follow x86-64 calling convention for both BPF-to-BPF and
> +	 * kfunc calls:
> +	 *   - Arg 6 is passed in R9 register
> +	 *   - Args 7+ are passed on the stack at [rsp]

Can the use of the R9 register cause data corruption across function calls?

The x86-64 C calling convention defines R9 as a caller-saved register.
However, the BPF verifier models argument 6 as stack memory, which implies
its value should be preserved across calls.

If a BPF program reads incoming argument 6 after making a helper or kfunc
call, will it receive a clobbered value instead of the original argument?
Similarly, could a helper call scheduled by LLVM between the setup of
outgoing argument 6 and the target call corrupt the R9 register?

[ ... ]

> +	/*
> +	 * Base offset from rbp for translating BPF outgoing args 7+
> +	 * to native offsets. BPF uses negative offsets from r11
> +	 * (r11-8 for arg6, r11-16 for arg7, ...) while x86 uses
> +	 * positive offsets from rsp ([rsp+0] for arg7, [rsp+8] for
> +	 * arg8, ...). Arg 6 goes to R9 directly.
> +	 *
> +	 * The translation reverses direction:
> +	 *   native_off = outgoing_arg_base - outgoing_rsp - bpf_off - 16
> +	 *
> +	 * Note that tail_call_reachable is guaranteed to be false when
> +	 * stack args exist, so tcc pushes need not be accounted for.
> +	 */

Does this formula correctly handle multi-byte sub-slot accesses?

Since the formula subtracts the BPF offset, the memory layout within the
8-byte slots appears mathematically reversed. While this correctly maps the
8-byte slot bases (e.g., -16 maps to rsp + 0), it seems to point to the
wrong memory for partial writes.

For example, if a program writes to the upper 32 bits of Arg 7 at BPF offset
-12, the formula yields BASE - (-12) - 16 = BASE - 4. Will this map the
access out-of-bounds below the stack pointer instead of to [rsp + 4]?

[ ... ]

> @@ -2129,12 +2222,26 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image, u8 *rw_image
>  				EMIT1(0xC7);
>  			goto st;
>  		case BPF_ST | BPF_MEM | BPF_DW:
> +			if (dst_reg == BPF_REG_PARAMS && insn->off == -8) {
> +				/* Arg 6: store immediate in r9 register */
> +				emit_mov_imm64(&prog, X86_REG_R9, imm32 >> 31, (u32)imm32);
> +				break;
> +			}
>  			EMIT2(add_1mod(0x48, dst_reg), 0xC7);
>  
> -st:			if (is_imm8(insn->off))
> -				EMIT2(add_1reg(0x40, dst_reg), insn->off);
> +st:			insn_off = insn->off;
> +			if (dst_reg == BPF_REG_PARAMS) {
> +				/* Args 7+: reverse BPF negative offsets to
> +				 * x86 positive rsp offsets.
> +				 * BPF off=-16 → [rsp+0], off=-24 → [rsp+8], ...
> +				 */
> +				insn_off = outgoing_arg_base - outgoing_rsp - insn_off - 16;
> +				dst_reg = BPF_REG_FP;
> +			}

Are the size constraints missing for Argument 6 here?

The check for Arg 6 (insn->off == -8) is only implemented under the BPF_DW
case. For sizes B, H, and W, execution will fall through to the st: label.
This would incorrectly translate the access as a normal stack operation
using the formula above, writing out-of-bounds below the stack pointer at
[rsp - 8] instead of populating R9.

Additionally, for BPF_LDX, a 1-byte load emits a full 64-bit move to R9.
Since the upper 56 bits of R9 are undefined for 1-byte arguments, could
they contain garbage data? This would bypass the verifier's strict value
bounds tracking, which assumes the loaded value is strictly within [0, 255].

[ ... ]

> @@ -2243,6 +2360,19 @@ st:			if (is_imm8(insn->off))
>  		case BPF_LDX | BPF_PROBE_MEMSX | BPF_H:
>  		case BPF_LDX | BPF_PROBE_MEMSX | BPF_W:
>  			insn_off = insn->off;
> +			if (src_reg == BPF_REG_PARAMS) {
> +				if (insn_off == 8) {
> +					/* Incoming arg 6: read from r9 */
> +					EMIT_mov(dst_reg, X86_REG_R9);
> +					break;
> +				}
> +				src_reg = BPF_REG_FP;
> +				/*
> +				 * Incoming args 7+: native_off == bpf_off
> +				 * (r11+16 → [rbp+16], r11+24 → [rbp+24], ...)
> +				 * No offset adjustment needed.
> +				 */
> +			}

Does this logic misroute stack accesses by assuming LDX always targets
incoming arguments and STX/ST always target outgoing arguments?

If a program writes an outgoing argument ([r11 - 16]) and then reads it back
via LDX, the JIT appears to apply the incoming argument translation mapping
the read to [rbp - 16] (the local stack) instead of [rsp] (the outgoing
area). This would return uninitialized data.

Conversely, if a program modifies an incoming argument ([r11 + 16]) using
STX, the JIT would apply the outgoing translation formula, yielding
BASE - 16 - 16 = BASE - 32, which writes out-of-bounds below the stack
pointer.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417034658.2625353-1-yonghong.song@linux.dev?part=13