Re: [PATCH bpf-next v7 4/9] bpf, x86: emit KASAN checks in x86 JITed programs

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 8/23/26 3:30 PM, Kumar Kartikeya Dwivedi wrote:
> On Sat Aug 22, 2026 at 1:33 AM CEST, sashiko-bot wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>> - [High] `emit_kasan_check` fails to save the caller-saved register %r10 (BPF_REG_AX), which corrupts BPF_REG_0 and memory pointers during atomic fetch operations.
>> --
>>
>> --- Patch [4]: [PATCH bpf-next v7 4/9] bpf, x86: emit KASAN checks in x86 JITed programs ---
>> commit 6bca50db6e9af9416363fe2e60f4e42f924950c9
>> Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>
>>
>> bpf, x86: emit KASAN checks in 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 13ba3232993ac..8134d028a2f2c 100644
>>> --- a/arch/x86/net/bpf_jit_comp.c
>>> +++ b/arch/x86/net/bpf_jit_comp.c
>>
>> [ ... ]
>>
>>> @@ -1110,6 +1121,92 @@ 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)
>>> +{
>>> +#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;
>>> +
>>> +	/* 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);
>>> +	/*
>>> +	 * SystemV ABI states that we should also save r10/r11, but in
>>> +	 * practice those registers are _not_ used by the limited set of
>>> +	 * kasan helpers we are calling here, so that's fine not to save those.
>>> +	 */
>>> +
>>> +	/* mov rdi, addr_reg */
>>
>> [Severity: High]
>> Is it safe to skip saving %r10 and %r11 here?
>>
>> In the x86 BPF JIT, %r10 is mapped to BPF_REG_AX. During atomic fetch
>> operations, BPF_REG_0 is stashed into BPF_REG_AX before calling
>> emit_kasan_check():
>>
>> arch/x86/net/bpf_jit_comp.c:do_jit() {
>>     ...
>>     /* Will need RAX as a CMPXCHG operand so save R0 */
>>     emit_mov_reg(&prog, true, BPF_REG_AX, BPF_REG_0);
>>     if (src_reg == BPF_REG_0)
>>         real_src_reg = BPF_REG_AX;
>>     if (dst_reg == BPF_REG_0)
>>         real_dst_reg = BPF_REG_AX;
>>
>>     if (!bpf_atomic_is_load_store(insn)) {
>>         if (!accesses_stack_only) {
>>             err = emit_kasan_check(env, &prog, real_dst_reg, insn, ip, true);
>>     ...
>> }
>>
>> Since the KASAN helpers are standard C functions, they are permitted by the
>> System V AMD64 ABI to clobber caller-saved registers like %r10 (especially if
>> kasan_report is called).
>>
>> If %r10 is clobbered, could this corrupt the stashed BPF_REG_0 and potentially
>> cause a crash when real_dst_reg is used as a memory pointer in the subsequent
>> emit_ldx() call?
>>
>> arch/x86/net/bpf_jit_comp.c:do_jit() {
>>     ...
>>     /* Load old value */
>>     emit_ldx(&prog, BPF_SIZE(insn->code), BPF_REG_0, real_dst_reg, insn->off);
>>     ...
>>     /* Restore R0 after clobbering RAX */
>>     emit_mov_reg(&prog, true, BPF_REG_0, BPF_REG_AX);
>>     ...
>> }
> 
> This looks like a real bug, R10 is used for both atomics and constant blinding.
> We probably also should save R11.


Hi Kumar, Alexis.

This topic has a bit of history now [1][2].
I don't quite like it, but IMO we should give in to the bots.

Reasons to save r10 and r11:
* SystemV ABI
* bots wouldn't shut up about it (think of future llm bot reports!)
* if our analysis of the kasan helpers is wrong about r10/r11 usage OR
  if something changes (the helpers themselves, the compilers, build
  flags, JIT's usage of r10/r11 etc.) - we'll get clobbered regs

Reasons to *not* save r10 and r11:
* we make the bpf prog a bit smaller and faster, the win is not very
  clear but it's percentages, not a factor

A counter-argument to clobbering is that a kasan bug here most likely
means a bug in the verifier, so it doesn't matter if machine dies.

A counter-argument to that would be: why deliberately increase the bug
surface and let the machine die if we can easily prevent it?

Pasting below a clobbering reproducer from my clanker.

[1] https://lore.kernel.org/all/5f38c9a5-a8a3-4bed-bb8c-b7260a1c1a11@linux.dev/
[2] https://lore.kernel.org/all/CAADnVQ+c9h_wuNwj8pjx885oNErGY7bxxCwKi+DiJ0XKSpyYfg@mail.gmail.com/


diff --git a/tools/testing/selftests/bpf/prog_tests/kasan.c b/tools/testing/selftests/bpf/prog_tests/kasan.c
index 2b424767a0f3..160445395ddc 100644
--- a/tools/testing/selftests/bpf/prog_tests/kasan.c
+++ b/tools/testing/selftests/bpf/prog_tests/kasan.c
@@ -309,6 +309,149 @@ static void run_blinding_subtest(void)
 	free(ctx);
 }
 
+/*
+ * Value-integrity subtests.
+ *
+ * The suite above only checks that a KASAN report was emitted. A report is not
+ * fatal here (the kernel runs with kasan_multi_shot and panic_on_warn=0), so
+ * the program keeps executing afterwards and its state must be intact. These
+ * two subtests check the two places where the JIT keeps live BPF state in
+ * BPF_REG_AX (x86 r10) across the injected __asan_* call.
+ */
+#define R0_SENTINEL	0x5EEDFACE
+#define ST_SENTINEL	0x0B0BCAFE
+
+static void run_atomic_fetch_integrity_subtest(struct test_ctx *ctx,
+					       struct kasan *skel)
+{
+	struct test_spec spec = {
+		.prog_type = "atomic_fetch_r0_integrity",
+		.is_write = true,
+	};
+
+	if (!test__start_subtest("atomic_fetch_r0_integrity"))
+		return;
+
+	strncpy(ctx->prog_name, "atomic_fetch_r0_integrity", PROG_NAME_MAX_LEN);
+
+	/*
+	 * Control run: same instruction, same instrumentation, unpoisoned
+	 * target. No report, so nothing may be disturbed. If this one fails,
+	 * the test itself is wrong.
+	 */
+	skel->data->poison_target = false;
+	skel->bss->r0_after_atomic = 0;
+	spec.expect_no_report = true;
+	exec_subtest(ctx, &spec, 8, false);
+	ASSERT_EQ(skel->bss->r0_after_atomic, R0_SENTINEL,
+		  "control: r0 preserved when no report fires");
+
+	skel->data->poison_target = true;
+	skel->bss->r0_after_atomic = 0;
+	spec.expect_no_report = false;
+
+	/* Asserts that the report fired: the check must have been emitted. */
+	exec_subtest(ctx, &spec, 8, false);
+
+	/*
+	 * ... and that returning from it left r0 alone. The JIT saves r0 into
+	 * BPF_REG_AX before the CMPXCHG loop and restores it after, with the
+	 * KASAN check in between.
+	 */
+	ASSERT_EQ(skel->bss->r0_after_atomic, R0_SENTINEL,
+		  "r0 preserved across a KASAN report in a fetch atomic");
+}
+
+static void run_atomic_fetch_addr_subtest(struct test_ctx *ctx,
+					  struct kasan *skel)
+{
+	struct test_spec spec = {
+		.prog_type = "atomic_fetch_r0_addr",
+		.is_write = true,
+	};
+
+	if (!test__start_subtest("atomic_fetch_r0_addr"))
+		return;
+
+	strncpy(ctx->prog_name, "atomic_fetch_r0_addr", PROG_NAME_MAX_LEN);
+	exec_subtest(ctx, &spec, 8, false);
+
+	ASSERT_EQ(skel->bss->r0_addr_after, skel->bss->r0_addr_ref,
+		  "r10 (used as the atomic base address) preserved");
+}
+
+static void run_atomic_fetch_oob_c_subtest(struct test_ctx *ctx)
+{
+	struct test_spec spec = {
+		.prog_type = "atomic_fetch_oob_c",
+		.is_write = true,
+	};
+
+	if (!test__start_subtest("atomic_fetch_oob_c"))
+		return;
+
+	strncpy(ctx->prog_name, "atomic_fetch_oob_c", PROG_NAME_MAX_LEN);
+	exec_subtest(ctx, &spec, 8, false);
+}
+
+static void run_blinding_integrity_subtest(void)
+{
+	struct test_spec spec = {
+		.prog_type = "st_blinded_integrity",
+		.is_write = true,
+	};
+	char bpf_jit_harden = '2';
+	struct kasan_write_val val;
+	struct kasan_harden *skel;
+	struct test_ctx *ctx;
+	__u32 key = 0;
+	int ret;
+
+	if (!test__start_subtest("st_blinded_integrity"))
+		return;
+
+	ctx = calloc(1, sizeof(*ctx));
+	if (!ASSERT_OK_PTR(ctx, "alloc blinding ctx"))
+		return;
+	ctx->klog_fd = -1;
+
+	ret = set_bpf_jit_harden(&bpf_jit_harden);
+	if (!ASSERT_OK(ret, "set bpf_jit_harden"))
+		goto free_ctx;
+
+	skel = kasan_harden__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open and load blinded prog"))
+		goto restore;
+
+	ctx->klog_fd = open_kernel_logs();
+	if (!ASSERT_OK_FD(ctx->klog_fd, "open kernel logs"))
+		goto destroy;
+
+	ctx->obj = skel->obj;
+	strncpy(ctx->prog_name, "st_blinded_integrity", PROG_NAME_MAX_LEN);
+
+	/* Asserts that the report fired. */
+	exec_subtest(ctx, &spec, 8, false);
+
+	/* ... and that the blinded store still wrote the right value. */
+	ret = bpf_map__lookup_elem(skel->maps.test_map, &key, sizeof(key),
+				   &val, sizeof(val), 0);
+	if (!ASSERT_OK(ret, "read back blinded store target"))
+		goto close_klog;
+	ASSERT_EQ(val.data_8, ST_SENTINEL,
+		  "blinded store value preserved across a KASAN report");
+
+close_klog:
+	close(ctx->klog_fd);
+destroy:
+	kasan_harden__destroy(skel);
+restore:
+	ASSERT_OK(set_bpf_jit_harden(&bpf_jit_harden),
+		  "restore hardening configuration");
+free_ctx:
+	free(ctx);
+}
+
 static struct test_spec tests[] = {
 	{
 		.prog_type = "st",
@@ -439,11 +582,16 @@ void test_kasan(void)
 		run_subtest(ctx, test);
 	}
 
+	run_atomic_fetch_integrity_subtest(ctx, skel);
+	run_atomic_fetch_addr_subtest(ctx, skel);
+	run_atomic_fetch_oob_c_subtest(ctx);
+
 	/*
 	 * Blinding subtest is handled differently as it needs the
 	 * corresponding program to be loaded with bpf_jit_harden raised
 	 */
 	run_blinding_subtest();
+	run_blinding_integrity_subtest();
 
 close:
 	close(ctx->klog_fd);
diff --git a/tools/testing/selftests/bpf/progs/kasan.c b/tools/testing/selftests/bpf/progs/kasan.c
index ea29197646b0..68a041f14353 100644
--- a/tools/testing/selftests/bpf/progs/kasan.c
+++ b/tools/testing/selftests/bpf/progs/kasan.c
@@ -459,4 +459,111 @@ int ldx_oob(struct __sk_buff *skb)
 	return tmp.data_1;
 }
 
+/*
+ * Value-integrity tests: a KASAN report must not change what the program
+ * computes. The kernel keeps running after a report under kasan_multi_shot,
+ * so every register the JIT holds live across the injected __asan_* call must
+ * come back unchanged.
+ */
+#define R0_SENTINEL	0x5EEDFACE
+
+/*
+ * The JIT lowers an RMW fetch atomic into a CMPXCHG loop and needs RAX as the
+ * CMPXCHG operand, so it stashes BPF r0 into BPF_REG_AX (x86 r10) before the
+ * loop and restores it afterwards. The KASAN check sits between the two, so
+ * r10 holds live BPF state across the call.
+ *
+ * Keep a sentinel in r0 across the atomic and publish it afterwards: if the
+ * check clobbers r10, the restored r0 is garbage.
+ */
+__u64 r0_after_atomic = 0;
+/* Cleared by the control run, which must not produce a report at all. */
+bool poison_target = true;
+
+SEC("tcx/ingress")
+int atomic_fetch_r0_integrity(struct __sk_buff *skb)
+{
+	struct kasan_test_val *val;
+	__u32 key = 0;
+
+	val = bpf_map_lookup_elem(&test_map, &key);
+	if (!val)
+		return 0;
+
+	if (poison_target)
+		bpf_kfunc_kasan_poison(val, sizeof(struct kasan_test_val));
+	asm volatile ("r0 = %[sentinel];"
+		      "r1 = 8;"
+		      "r1 = atomic_fetch_or((u64 *)(%[val] + 8), r1);"
+		      "*(u64 *)(%[out] + 0) = r0;"
+		      :
+		      : [val] "r" (val), [out] "r" (&r0_after_atomic),
+			[sentinel] "i" (R0_SENTINEL)
+		      : "r0", "r1", "memory");
+	if (poison_target)
+		bpf_kfunc_kasan_unpoison(val, sizeof(struct kasan_test_val));
+
+	return 0;
+}
+
+/*
+ * Same lowering, but with the target pointer in r0. The JIT then uses
+ * BPF_REG_AX as real_dst_reg, i.e. r10 is not merely holding a value across
+ * the KASAN check - it is the base address of the ldx and of the locked
+ * cmpxchg that follow it.
+ */
+__u64 r0_addr_after = 0;
+__u64 r0_addr_ref = 0;
+
+SEC("tcx/ingress")
+int atomic_fetch_r0_addr(struct __sk_buff *skb)
+{
+	struct kasan_test_val *val;
+	__u32 key = 0;
+
+	val = bpf_map_lookup_elem(&test_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_kfunc_kasan_poison(val, sizeof(struct kasan_test_val));
+	asm volatile ("r0 = %[val];"
+		      "r1 = 8;"
+		      "r1 = atomic_fetch_or((u64 *)(r0 + 8), r1);"
+		      "*(u64 *)(%[out] + 0) = r0;"
+		      "*(u64 *)(%[ref] + 0) = %[val];"
+		      :
+		      : [val] "r" (val), [out] "r" (&r0_addr_after),
+			[ref] "r" (&r0_addr_ref)
+		      : "r0", "r1", "memory");
+	bpf_kfunc_kasan_unpoison(val, sizeof(struct kasan_test_val));
+
+	return 0;
+}
+
+/*
+ * Plain C, no asm: the second lookup returns the map value pointer in r0 and
+ * the compiler leaves it there for the fetch atomic, so the JIT uses
+ * BPF_REG_AX (r10) as the base address of the post-check ldx and cmpxchg.
+ */
+SEC("tcx/ingress")
+int atomic_fetch_oob_c(struct __sk_buff *skb)
+{
+	struct kasan_test_val *val, *val2;
+	__u32 key = 0;
+
+	val = bpf_map_lookup_elem(&test_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_kfunc_kasan_poison(val, sizeof(struct kasan_test_val));
+
+	val2 = bpf_map_lookup_elem(&test_map, &key);
+	if (!val2)
+		return 0;
+	__sync_fetch_and_or(&val2->data_8, 8);
+
+	bpf_kfunc_kasan_unpoison(val, sizeof(struct kasan_test_val));
+	return 0;
+}
+
 char LICENSE[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/kasan_harden.c b/tools/testing/selftests/bpf/progs/kasan_harden.c
index a2756bbfd529..9172a1017a8e 100644
--- a/tools/testing/selftests/bpf/progs/kasan_harden.c
+++ b/tools/testing/selftests/bpf/progs/kasan_harden.c
@@ -38,4 +38,36 @@ int st_blinded(struct __sk_buff *skb)
 	return 0;
 }
 
+/*
+ * Constant blinding lowers a BPF_ST_MEM into
+ *   BPF_REG_AX = imm ^ rnd; BPF_REG_AX ^= rnd; *(dst + off) = BPF_REG_AX
+ * and the JIT emits the KASAN check before that store, so BPF_REG_AX (x86 r10)
+ * holds the value to be written across the __asan_* call.
+ *
+ * Written in asm to guarantee a raw BPF_ST opcode regardless of -mcpu: at
+ * -mcpu=v3 clang lowers an immediate store to MOV+STX, which is not blinded
+ * through BPF_REG_AX and does not exercise this path.
+ */
+#define ST_SENTINEL	0x0B0BCAFE
+
+SEC("tcx/ingress")
+int st_blinded_integrity(struct __sk_buff *skb)
+{
+	struct kasan_test_val *val;
+	__u32 key = 0;
+
+	val = bpf_map_lookup_elem(&test_map, &key);
+	if (!val)
+		return 0;
+
+	bpf_kfunc_kasan_poison(val, sizeof(struct kasan_test_val));
+	asm volatile ("*(u64 *)(%[val] + 8) = %[sentinel];"
+		      :
+		      : [val] "r" (val), [sentinel] "i" (ST_SENTINEL)
+		      : "memory");
+	bpf_kfunc_kasan_unpoison(val, sizeof(struct kasan_test_val));
+
+	return 0;
+}
+
 char LICENSE[] SEC("license") = "GPL";
-- 
2.53.0-Meta