Re: [PATCH bpf-next v5 06/10] bpf, x86: emit KASAN checks into x86 JITed programs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] KASAN checks clobber caller-saved registers %r10 (BPF_REG_AX) and %r11 (AUX_REG), leading to memory corruption
--

--- Patch [6]: [PATCH bpf-next v5 06/10] bpf, x86: emit KASAN checks into x86 JITed programs ---
commit e841cd22d378b4b1ec9b5d8608f051fedbf3837f
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

bpf, x86: emit KASAN checks into x86 JITed programs

This commit inserts KASAN shadow memory checks before memory load and store
operations in JIT-compiled BPF programs. This helps detect memory safety
bugs such as use-after-free and out-of-bounds accesses at runtime.

> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
[ ... ]
> @@ -2605,15 +2640,16 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
>  			}
>  			fallthrough;
>  		case BPF_STX | BPF_ATOMIC | BPF_W:
> -		case BPF_STX | BPF_ATOMIC | BPF_DW:
> -			if (insn->imm == (BPF_AND | BPF_FETCH) ||
> -			    insn->imm == (BPF_OR | BPF_FETCH) ||
> -			    insn->imm == (BPF_XOR | BPF_FETCH)) {
> -				bool is64 = BPF_SIZE(insn->code) == BPF_DW;
> -				u32 real_src_reg = src_reg;
> -				u32 real_dst_reg = dst_reg;
> -				u8 *branch_target;
> -
> +		case BPF_STX | BPF_ATOMIC | BPF_DW: {
> +			bool is64 = BPF_SIZE(insn->code) == BPF_DW;
> +			u32 real_src_reg = src_reg;
> +			u32 real_dst_reg = dst_reg;
> +			u8 *branch_target;
> +			bool is_atomic_fetch =
> +				(insn->imm == (BPF_AND | BPF_FETCH) ||
> +				 insn->imm == (BPF_OR | BPF_FETCH) ||
> +				 insn->imm == (BPF_XOR | BPF_FETCH));
> +			if (is_atomic_fetch) {
>  				/*
>  				 * Can't be implemented with a single x86 insn.
>  				 * Need to do a CMPXCHG loop.
> @@ -2626,7 +2662,17 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
>  				if (dst_reg == BPF_REG_0)
>  					real_dst_reg = BPF_REG_AX;
>  
> +				ip += 3;
> +			}
> +			if (!bpf_atomic_is_load_store(insn)) {
> +				err = emit_kasan_check(env, &prog, real_dst_reg,
> +						       insn, ip, true,
> +						       accesses_stack_only);

[Severity: Critical]
Does this sequence clobber the temporarily saved BPF_REG_0 register?

The CMPXCHG loop saves BPF_REG_0 into BPF_REG_AX just before this block:

    /* Will need RAX as a CMPXCHG operand so save R0 */
    emit_mov_reg(&prog, true, BPF_REG_AX, BPF_REG_0);

Immediately after, emit_kasan_check() emits a C function call to the KASAN
runtime. However, emit_kasan_check() does not push and pop BPF_REG_AX or
AUX_REG in its register save sequence.

Since those registers map to %r10 and %r11, which are caller-saved in the
x86-64 System V ABI, the KASAN C functions can overwrite them. The later
restore of BPF_REG_0 from BPF_REG_AX would then yield a garbage value.

Could this lead to memory corruption, especially if the verifier also
utilizes BPF_REG_AX for constant blinding across BPF_ST instructions which
are similarly instrumented by emit_kasan_check()?

> +				if (err)
> +					return err;
>  				branch_target = prog;
> +			}

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