Re: [PATCH bpf-next v5 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 and restore caller-saved registers `%r10` and `%r11`, causing ABI violation and memory corruption.
- [Low] `emit_kasan_check()` is static but unreferenced, breaking the build, and KASAN configuration is incomplete.
--

--- Patch [4]: [PATCH bpf-next v5 04/10] bpf, x86: add helper to emit kasan checks in x86 JITed programs ---
commit 95547145be173ba9a87f9eff2492384b8ac25f18
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(), which emits KASAN shadow memory checks
before memory accesses in JIT-compiled BPF programs. The helper calls standard
KASAN C functions to validate the memory accesses.

> 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]
Is this static function used anywhere in this patch? It looks like it is
declared and defined but never referenced, which will cause a build failure
when compiled with -Werror=unused-function.

Additionally, the HAVE_EBPF_JIT_KASAN Kconfig option does not appear to be
selected by any architecture in this series, making the feature unreachable.
(Also noting a potential issue where adjust_insn_aux_data() in
kernel/bpf/fixups.c may retain stale state when insn_off_in_patch == -1).

> +#ifdef CONFIG_BPF_JIT_KASAN
> +	u32 bpf_size = BPF_SIZE(insn->code);
> +	s32 off = insn->off;

[ ... ]

> +	/* 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]
Does skipping the save and restore of %r10 and %r11 violate the System V ABI?

While the commit message notes they are skipped to reduce overhead, %r10 is
mapped to BPF_REG_AX in the x86 JIT. Because standard C functions like the
KASAN helpers clobber caller-saved registers, wouldn't this corrupt the
BPF_REG_AX register (which is used for operations like blinded constant
stores) when the KASAN check is executed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709-kasan-v5-0-1c64af8e4e1e@bootlin.com?part=4