Re: [PATCH RFC bpf-next 4/8] bpf, x86: add helper to emit kasan checks in x86 JITed programs

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Missing call depth accounting for Retbleed mitigations leads to call depth counter underflow.
- [Medium] BPF_ST instructions are incorrectly classified as memory reads instead of writes.
- [Low] Flawed stack realignment logic wastes stack space.
--

commit d2f06b6218cd74384e59faf159117526fab479e6
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

bpf, x86: add helper to emit kasan checks in x86 JITed programs

This commit adds emit_kasan_check() to emit KASAN shadow memory checks
before memory accesses in x86 JIT-compiled BPF programs. It saves registers,
computes the accessed address, calls the relevant KASAN load/store function,
and restores the registers.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index ea9e707e8abff..b90103bd00809 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[ ... ]

> @@ -1301,6 +1305,95 @@ static void emit_store_stack_imm64(u8 **pprog, int reg, int stack_off, u64 imm64
>  	emit_stx(pprog, BPF_DW, BPF_REG_FP, reg, stack_off);
>  }
>  
> +static int emit_kasan_check(u8 **pprog, u32 addr_reg, struct bpf_insn *insn,
> +			    u8 *ip, bool accesses_stack)
> +{
> +#ifdef CONFIG_BPF_JIT_KASAN
> +	bool is_write = BPF_CLASS(insn->code) == BPF_STX;

Does this code incorrectly classify BPF_ST instructions as memory reads?

The BPF instruction set includes the BPF_ST class for storing immediate
values into memory. Because BPF_CLASS(insn->code) for BPF_ST evaluates to
0x02, the is_write condition evaluates to false. This causes the helper to
inject KASAN load checks instead of store checks, leading to inaccurate
error reporting.

> +	u32 bpf_size = BPF_SIZE(insn->code);
> +	s32 off = insn->off;
> +	u8 *prog = *pprog;
> +	void *kasan_func;
> +
> +	if (accesses_stack)
> +		return 0;
> +
> +	/* Derive KASAN check function from access type and size */
> +	switch (bpf_size) {

[ ... ]

> +	/* Save r10 */
> +	EMIT2(0x41, 0x52);
> +	/* Save r11 */
> +	EMIT2(0x41, 0x53);
> +	/* We have pushed 72 bytes, realign stack to 16 bytes: sub rsp, 8 */
> +	EMIT4(0x48, 0x83, 0xEC, 8);

Is this stack realignment logic correct?

The x86 BPF JIT prologue pushes a variable number of callee-saved registers
and subtracts an 8-byte aligned stack_depth, meaning the stack pointer can
be 8-byte misaligned prior to the check. If the stack is initially 8-byte
misaligned, pushing 72 bytes and subtracting 8 preserves the 8-byte
misalignment. Because the Linux kernel is compiled with 8-byte stack
alignment, the sub rsp, 8 instruction seems unnecessary and wastes 8 bytes
of stack space.

> +
> +	/* mov rdi, addr_reg */
> +	EMIT_mov(BPF_REG_1, addr_reg);
> +
> +	/* add rdi, off (if offset is non-zero) */
> +	if (off) {

[ ... ]

> +	/* Adjust ip to account for the instrumentation generated so far */
> +	ip += (prog - *pprog);
> +	/* call kasan_func */
> +	if (emit_call(&prog, kasan_func, ip))
> +		return -ERANGE;

Does this call need call depth accounting for Retbleed mitigations?

When CONFIG_CALL_DEPTH_TRACKING is enabled, every call instruction executed
in the kernel must be properly accounted for to increment the call depth
counter. Other dynamically emitted calls in the BPF JIT explicitly emit
this tracking via x86_call_depth_emit_accounting() prior to emitting the
actual call.

By omitting this accounting, the call to the KASAN function will not
increment the call depth counter. When the KASAN function returns, its
return thunk will decrement the counter, causing it to underflow. This
causes the return thunks to assume the Return Stack Buffer is empty and
trigger expensive RSB stuffing on subsequent returns.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260413-kasan-v1-0-1a5831230821@bootlin.com?part=4