bpf: zero extend the result of an arena 32-bit cmpxchg

PATCH Applied Eduard Zingerman 3 participants 6 messages Sep 3 – Sep 4

Subsystems (4)

Kselftest BPF BPF selftests bpftool

lore

Share & tools

lore

Eduard Zingerman

author maintainer

2026-09-03 17:15 UTC #0 lore raw

PATCH 1/3 bpf: zero extend the result of an arena 32-bit cmpxchg

bpf_convert_ctx_accesses() rewrites an atomic on an arena pointer from BPF_STX | BPF_ATOMIC to BPF_STX | BPF_PROBE_ATOMIC, and it runs before bpf_opt_subreg_zext_lo32_rnd_hi32().

That pass emits an explicit zero extension for a 32-bit cmpxchg even when bpf_jit_needs_zext() is false. This is done because on some architectures 32-bit cmpxchg requires explicit zero extension for the dst register. E.g. on x86-64 'lock cmpxchg' does not change the %eax if comparison is successful, while BPF semantics declare that each operation on a 32-bit register zero extends it's upper half.

is_cmpxchg_insn() matches BPF_MODE == BPF_ATOMIC only, so an arena cmpxchg misses said zero extension adjustment. This patch adjusts is_cmpxchg_insn() to match BPF_PROBE_ATOMIC alongside BPF_ATOMIC.

Fixes d503a04f8bc0 ("bpf: Add support for certain atomics in bpf_arena to x86 JIT")

Reported-by Nicholas Carlini <[email protected]>

Signed-off-by Eduard Zingerman <[email protected]>

---
 kernel/bpf/fixups.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

kernel/bpf/fixups.c +6 -1

index 65b441e4a351..52d3cec33672 100644
@@ -13,10 +13,15 @@
 
 #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
 
+/*
+ * Matches BPF_PROBE_ATOMIC too: bpf_convert_ctx_accesses() rewrites arena
+ * atomics before bpf_opt_subreg_zext_lo32_rnd_hi32() runs.
+ */
 static bool is_cmpxchg_insn(const struct bpf_insn *insn)
 {
 	return BPF_CLASS(insn->code) == BPF_STX &&
-	       BPF_MODE(insn->code) == BPF_ATOMIC &&
+	       (BPF_MODE(insn->code) == BPF_ATOMIC ||
+		BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) &&
 	       insn->imm == BPF_CMPXCHG;
 }

Signature · 1 line

2.55.0

Eduard Zingerman

author maintainer

2026-09-03 17:15 UTC #1 lore raw

PATCH 2/3 bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics

bpf_convert_ctx_accesses() rewrites an atomic on an arena pointer from BPF_STX | BPF_ATOMIC to BPF_STX | BPF_PROBE_ATOMIC, this patch adjusts print_bpf_insn() to print such instructions as regular atomics with a 'probe_' prefix (instead of printing them as BUG_XX).

Signed-off-by Eduard Zingerman <[email protected]>

---
 kernel/bpf/disasm.c | 47 ++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

kernel/bpf/disasm.c +25 ~~-22~~

index b1a3fbe3fda5..3ce8d74b0e40 100644
@@ -7,6 +7,9 @@
 
 #include "disasm.h"
 
+/* Only defined by the non-UAPI linux/filter.h, which this file cannot use. */
+#define BPF_PROBE_ATOMIC 0xe0
+
 #define __BPF_FUNC_STR_FN(x) [BPF_FUNC_ ## x] = __stringify(bpf_ ## x)
 static const char * const func_id_str[] = {
 	__BPF_FUNC_MAPPER(__BPF_FUNC_STR_FN)

@@ -226,57 +229,57 @@ void print_bpf_insn(const struct bpf_insn_cbs *cbs,
 				insn->imm);
 		}
 	} else if (class == BPF_STX) {
+		const char *probe_pfx = BPF_MODE(insn->code) == BPF_PROBE_ATOMIC ? "probe " : "";
+		bool atomic = BPF_MODE(insn->code) == BPF_ATOMIC ||
+			      BPF_MODE(insn->code) == BPF_PROBE_ATOMIC;
+
 		if (BPF_MODE(insn->code) == BPF_MEM)
 			verbose(cbs->private_data, "(%02x) *(%s *)(r%d %+d) = r%d",
 				insn->code,
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg,
 				insn->off, insn->src_reg);
-		else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
+		else if (atomic &&
 			 (insn->imm == BPF_ADD || insn->imm == BPF_AND ||
 			  insn->imm == BPF_OR || insn->imm == BPF_XOR)) {
-			verbose(cbs->private_data, "(%02x) lock *(%s *)(r%d %+d) %s r%d",
-				insn->code,
+			verbose(cbs->private_data, "(%02x) %slock *(%s *)(r%d %+d) %s r%d",
+				insn->code, probe_pfx,
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg, insn->off,
 				bpf_alu_string[BPF_OP(insn->imm) >> 4],
 				insn->src_reg);
-		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
+		} else if (atomic &&
 			   (insn->imm == (BPF_ADD | BPF_FETCH) ||
 			    insn->imm == (BPF_AND | BPF_FETCH) ||
 			    insn->imm == (BPF_OR | BPF_FETCH) ||
 			    insn->imm == (BPF_XOR | BPF_FETCH))) {
-			verbose(cbs->private_data, "(%02x) r%d = atomic%s_fetch_%s((%s *)(r%d %+d), r%d)",
-				insn->code, insn->src_reg,
+			verbose(cbs->private_data, "(%02x) %sr%d = atomic%s_fetch_%s((%s *)(r%d %+d), r%d)",
+				insn->code, probe_pfx, insn->src_reg,
 				BPF_SIZE(insn->code) == BPF_DW ? "64" : "",
 				bpf_atomic_alu_string[BPF_OP(insn->imm) >> 4],
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg, insn->off, insn->src_reg);
-		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
-			   insn->imm == BPF_CMPXCHG) {
-			verbose(cbs->private_data, "(%02x) r0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)",
-				insn->code,
+		} else if (atomic && insn->imm == BPF_CMPXCHG) {
+			verbose(cbs->private_data, "(%02x) %sr0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)",
+				insn->code, probe_pfx,
 				BPF_SIZE(insn->code) == BPF_DW ? "64" : "",
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg, insn->off,
 				insn->src_reg);
-		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
-			   insn->imm == BPF_XCHG) {
-			verbose(cbs->private_data, "(%02x) r%d = atomic%s_xchg((%s *)(r%d %+d), r%d)",
-				insn->code, insn->src_reg,
+		} else if (atomic && insn->imm == BPF_XCHG) {
+			verbose(cbs->private_data, "(%02x) %sr%d = atomic%s_xchg((%s *)(r%d %+d), r%d)",
+				insn->code, probe_pfx, insn->src_reg,
 				BPF_SIZE(insn->code) == BPF_DW ? "64" : "",
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg, insn->off, insn->src_reg);
-		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
-			   insn->imm == BPF_LOAD_ACQ) {
-			verbose(cbs->private_data, "(%02x) r%d = load_acquire((%s *)(r%d %+d))",
-				insn->code, insn->dst_reg,
+		} else if (atomic && insn->imm == BPF_LOAD_ACQ) {
+			verbose(cbs->private_data, "(%02x) %sr%d = load_acquire((%s *)(r%d %+d))",
+				insn->code, probe_pfx, insn->dst_reg,
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->src_reg, insn->off);
-		} else if (BPF_MODE(insn->code) == BPF_ATOMIC &&
-			   insn->imm == BPF_STORE_REL) {
-			verbose(cbs->private_data, "(%02x) store_release((%s *)(r%d %+d), r%d)",
-				insn->code,
+		} else if (atomic && insn->imm == BPF_STORE_REL) {
+			verbose(cbs->private_data, "(%02x) %sstore_release((%s *)(r%d %+d), r%d)",
+				insn->code, probe_pfx,
 				bpf_ldst_string[BPF_SIZE(insn->code) >> 3],
 				insn->dst_reg, insn->off, insn->src_reg);
 		} else {

Signature · 1 line

2.55.0

[email protected]

AI review

2026-09-03 18:11 UTC #2 lore raw

Re: [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics

The changelog says the instructions are printed "as regular atomics with a 'probe_' prefix", but looking at the implementation:

Quoted 6 lines

bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
bpf_convert_ctx_accesses() rewrites an atomic on an arena pointer from BPF_STX | BPF_ATOMIC to BPF_STX | BPF_PROBE_ATOMIC, this patch adjusts print_bpf_insn() to print such instructions as regular atomics with a 'probe_' prefix (instead of printing them as BUG_XX).

The changelog says the instructions are printed "as regular atomics with a 'probe_' prefix", but looking at the implementation:

    const char *probe_pfx = BPF_MODE(insn->code) == BPF_PROBE_ATOMIC ? "probe " : "";
    ...
    verbose(cbs->private_data, "(%02x) %sr0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)",
            insn->code, probe_pfx, ...);

the code emits a separate word "probe " (trailing space, no underscore) at the start of the rendered instruction, producing output like:

    probe r0 = atomic_cmpxchg((u32 *)(r1 +0), r0, r2)

rather than:

    r0 = probe_atomic_cmpxchg(...)

as the message suggests. For the fetch/cmpxchg/xchg/load_acquire/ store_release forms, this also detaches the qualifier from the operation it qualifies (e.g. "probe r%d = atomic..._fetch_...", reading as if "probe" modified the destination register).

Should the changelog describe the emitted "probe " prefix, or should the prefix be attached to the operation name (probe_atomic_cmpxchg, probe_load_acquire, etc.) to match the message?

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33784588558

Eduard Zingerman

author maintainer

2026-09-03 19:38 UTC #3 lore raw

Re: [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics

Quoted 24 lines · On Thu, 2026-09-03 at 18:11 +0000, [email protected] wrote:

Quoted 6 lines
bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
bpf_convert_ctx_accesses() rewrites an atomic on an arena pointer from BPF_STX | BPF_ATOMIC to BPF_STX | BPF_PROBE_ATOMIC, this patch adjusts print_bpf_insn() to print such instructions as regular atomics with a 'probe_' prefix (instead of printing them as BUG_XX).
The changelog says the instructions are printed "as regular atomics with a 'probe_' prefix", but looking at the implementation:
    const char *probe_pfx = BPF_MODE(insn->code) == BPF_PROBE_ATOMIC ? "probe " : "";
    ...
    verbose(cbs->private_data, "(%02x) %sr0 = atomic%s_cmpxchg((%s *)(r%d %+d), r0, r%d)", insn->code, probe_pfx, ...);
the code emits a separate word "probe " (trailing space, no underscore) at the start of the rendered instruction, producing output like:
    probe r0 = atomic_cmpxchg((u32 *)(r1 +0), r0, r2)
rather than:
    r0 = probe_atomic_cmpxchg(...)

Quoted 8 lines

as the message suggests. For the fetch/cmpxchg/xchg/load_acquire/ store_release forms, this also detaches the qualifier from the operation it qualifies (e.g. "probe r%d = atomic..._fetch_...", reading as if "probe" modified the destination register).
Should the changelog describe the emitted "probe " prefix, or should the prefix be attached to the operation name (probe_atomic_cmpxchg, probe_load_acquire, etc.) to match the message?

Yes, I should have updated the log. The intent is to print:

(hex) probe ...something-something...

will respin v2, waiting for sashiko.

Eduard Zingerman

author maintainer

2026-09-03 17:15 UTC #4 lore raw

PATCH 3/3 selftests/bpf: check zero extension of an arena 32-bit cmpxchg

In reply to Eduard Zingerman

Add a test to verify that destination register of a 32-bit cmpxchg operating on an arena pointer is explicitly zero extended. W/o patch #1 this did not happen.

Reported-by Nicholas Carlini <[email protected]>

Signed-off-by Eduard Zingerman <[email protected]>

---
 .../selftests/bpf/progs/verifier_zext.c       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

tools/testing/selftests/bpf/progs/verifier_zext.c +26 -0

index 8f2362da91d6..572017fe28fb 100644
@@ -356,6 +356,32 @@ __naked void arena_ptr(void)
 	: __clobber_all);
 }
 
+/*
+ * Result of a 32-bit cmpxchg is always explicitly zero extended.
+ * Check that this holds for arenas (BPF_PROBE_ATOMIC instruction flavor).
+ */
+SEC("socket")
+__success
+__xlated("probe r0 = atomic_cmpxchg((u32 *)(r1 +0), r0, r2)")
+__xlated("w0 = w0")
+__naked void zext_arena_cmpxchg32(void)
+{
+	asm volatile ("					\
+	r9 = %[arena] ll;	/* associate the arena with the program */ \
+	r1 = 0;						\
+	r1 = addr_space_cast(r1, 0, 1);			\
+	r0 = 0;						\
+	r2 = 0;						\
+	.8byte %[cmpxchg32];				\
+	r0 >>= 32;		/* make the upper half live */ \
+	exit;						\
+"	:
+	: __imm_addr(arena),
+	  __imm_insn(cmpxchg32,
+		     BPF_ATOMIC_OP(BPF_W, BPF_CMPXCHG, BPF_REG_1, BPF_REG_2, 0))
+	: __clobber_all);
+}
+
 #endif
 
 /* Check if probe mem loads keep their zero extension. */

Signature · 1 line

2.55.0

[email protected]

CI Applied

2026-09-04 02:01 UTC #5 lore raw

In reply to Eduard Zingerman

This series was applied to bpf/bpf.git (master) by Alexei Starovoitov <[email protected]>:

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <[email protected]>:

Quoted 4 lines · On Thu, 3 Sep 2026 10:15:39 -0700 you wrote:

bpf_convert_ctx_accesses() rewrites an atomic on an arena pointer from BPF_STX | BPF_ATOMIC to BPF_STX | BPF_PROBE_ATOMIC, and it runs before bpf_opt_subreg_zext_lo32_rnd_hi32().

Quoted 8 lines

That pass emits an explicit zero extension for a 32-bit cmpxchg even when bpf_jit_needs_zext() is false. This is done because on some architectures 32-bit cmpxchg requires explicit zero extension for the dst register. E.g. on x86-64 'lock cmpxchg' does not change the %eax if comparison is successful, while BPF semantics declare that each operation on a 32-bit register zero extends it's upper half.
[...]

Here is the summary with links:
- [bpf,1/3] bpf: zero extend the result of an arena 32-bit cmpxchg

    https://git.kernel.org/bpf/bpf/c/4814ed6406f3
  - [bpf,2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
    https://git.kernel.org/bpf/bpf/c/1f3cd9719c40
  - [bpf,3/3] selftests/bpf: check zero extension of an arena 32-bit cmpxchg
    https://git.kernel.org/bpf/bpf/c/54ed91950363

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html