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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] `emit_kasan_check` fails to save and restore the caller-saved registers %r10 (`BPF_REG_AX`) and %r11 (`AUX_REG`). This corrupts state during atomic fetch operations and constant blinding, potentially leading to arbitrary memory corruption or kernel crashes.
- [Low] Atomic read-modify-write (RMW) operations are incorrectly instrumented as KASAN reads instead of KASAN writes.
--

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

bpf, x86: emit KASAN checks into x86 JITed programs

Insert 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.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 959fcaa0cde9a..65a00e84799cd 100644
> --- 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, false,
> +						       accesses_stack_only);

[Severity: Critical]
Does this KASAN check clobber %r10 (BPF_REG_AX) and %r11 (AUX_REG)?

If this is an atomic fetch operation and BPF_REG_0 is used, the JIT saves
BPF_REG_0 into BPF_REG_AX (%r10). However, the emit_kasan_check() function
emits a C function call without saving %r10 or %r11:

arch/x86/net/bpf_jit_comp.c:emit_kasan_check() {
	...
	/* Save r8 */
	EMIT2(0x41, 0x50);
	/* Save r9 */
	EMIT2(0x41, 0x51);

	/* mov rdi, addr_reg */
	EMIT_mov(BPF_REG_1, addr_reg);
	...
	/* call kasan_func */
	if (emit_call(&prog, kasan_func, ip))
		return -ERANGE;
	...
}

Since C functions can clobber %r10 under the System V ABI, could the
subsequent use of real_dst_reg (which points to %r10) lead to wild memory
accesses?

[Severity: Low]
Are atomic RMW operations incorrectly instrumented as KASAN reads?

The 6th argument to emit_kasan_check() is is_write. Here it is hardcoded to
false for mutating atomic RMW instructions. Should this be true instead, so
that KASAN diagnostics report out-of-bounds writes rather than out-of-bounds
reads?

> +				if (err)
> +					return err;
>  				branch_target = prog;
> +			}
> +			if (is_atomic_fetch) {
>  				/* Load old value */
>  				emit_ldx(&prog, BPF_SIZE(insn->code),
>  					 BPF_REG_0, real_dst_reg, insn->off);

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