Re: [PATCH bpf-next v4 04/10] bpf, x86: add helper to emit kasan checks in x86 JITed programs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] emit_kasan_check() fails to save caller-saved registers r10 (BPF_REG_AX) and r11 (AUX_REG), causing them to be clobbered by C KASAN helpers.
- [Low] The static function emit_kasan_check() is defined but never called, leading to a -Wunused-function compiler warning and build failure.
--

--- Patch [4]: [PATCH bpf-next v4 04/10] bpf, x86: add helper to emit kasan checks in x86 JITed programs ---
commit 702ec90119cefcab13a29595976ba78253e74ce6
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

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

This commit adds the emit_kasan_check() function for x86 JIT-compiled
BPF programs. It emits KASAN shadow memory checks before memory accesses
by saving state, computing the address, and calling C KASAN helpers.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index de7515ea1beae..d35f58350d710 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> @@ -1110,6 +1121,90 @@ static void maybe_emit_1mod(u8 **pprog, u32 reg, bool is64)
>  	*pprog = prog;
>  }
>  
> +static int emit_kasan_check(struct bpf_verifier_env *env, u8 **pprog,
> +			    u32 addr_reg, struct bpf_insn *insn, u8 *ip,
> +			    bool is_write, bool accesses_stack_only)
> +{

[Severity: Low]
Does this introduce a -Wunused-function compiler warning? The static function
emit_kasan_check() is added here but does not appear to be called anywhere in
this patch. This could cause build failures on setups with -Werror, breaking
git bisectability.

> +#ifdef CONFIG_BPF_JIT_KASAN
> +	u32 bpf_size = BPF_SIZE(insn->code);
> +	s32 off = insn->off;
> +	u8 *prog = *pprog;
> +	void *kasan_func;
> +
> +	if (!env)
> +		return 0;
> +
> +	if (accesses_stack_only)
> +		return 0;
> +
> +	/* Derive KASAN check function from access type and size */
> +	switch (bpf_size) {
> +	case BPF_B:
> +		kasan_func = is_write ? __asan_store1 : __asan_load1;
> +		break;
> +	case BPF_H:
> +		kasan_func = is_write ? __asan_store2 : __asan_load2;
> +		break;
> +	case BPF_W:
> +		kasan_func = is_write ? __asan_store4 : __asan_load4;
> +		break;
> +	case BPF_DW:
> +		kasan_func = is_write ? __asan_store8 : __asan_load8;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* Save rax */
> +	EMIT1(0x50);
> +	/* Save rcx */
> +	EMIT1(0x51);
> +	/* Save rdx */
> +	EMIT1(0x52);
> +	/* Save rsi */
> +	EMIT1(0x56);
> +	/* Save rdi */
> +	EMIT1(0x57);
> +	/* Save r8 */
> +	EMIT2(0x41, 0x50);
> +	/* Save r9 */
> +	EMIT2(0x41, 0x51);

[Severity: High]
Could omitting r10 and r11 here lead to data corruption? 

The commit message mentions skipping them to reduce overhead, but the called
C functions (__asan_loadX, __asan_storeX) follow the System V AMD64 ABI and
might clobber caller-saved registers like r10 and r11. 

In the x86 BPF JIT, r10 is mapped to BPF_REG_AX, which is used across
instructions to hold state (such as during constant blinding rewrites where
BPF_ST_MEM is transformed into BPF_STX_MEM using BPF_REG_AX as its source).

Would this result in storing garbage data when the C helper clobbers
BPF_REG_AX?

> +
> +	/* mov rdi, addr_reg */
> +	EMIT_mov(BPF_REG_1, addr_reg);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-kasan-v4-0-d5c177ab8227@bootlin.com?part=4