BPF List
 help / color / mirror / Atom feed
* [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg
@ 2026-09-03 17:15 Eduard Zingerman
  2026-09-03 17:15 ` [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics Eduard Zingerman
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-09-03 17:15 UTC (permalink / raw)
  To: bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc

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 <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 kernel/bpf/fixups.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 65b441e4a351..52d3cec33672 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -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;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
  2026-09-03 17:15 [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg Eduard Zingerman
@ 2026-09-03 17:15 ` Eduard Zingerman
  2026-09-03 18:11   ` bot+bpf-ci
  2026-09-03 17:15 ` [PATCH bpf 3/3] selftests/bpf: check zero extension of an arena 32-bit cmpxchg Eduard Zingerman
  2026-09-04  2:00 ` [PATCH bpf 1/3] bpf: zero extend the result " patchwork-bot+netdevbpf
  2 siblings, 1 reply; 6+ messages in thread
From: Eduard Zingerman @ 2026-09-03 17:15 UTC (permalink / raw)
  To: bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc

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 <eddyz87@gmail.com>
---
 kernel/bpf/disasm.c | 47 ++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c
index b1a3fbe3fda5..3ce8d74b0e40 100644
--- a/kernel/bpf/disasm.c
+++ b/kernel/bpf/disasm.c
@@ -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 {
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH bpf 3/3] selftests/bpf: check zero extension of an arena 32-bit cmpxchg
  2026-09-03 17:15 [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg Eduard Zingerman
  2026-09-03 17:15 ` [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics Eduard Zingerman
@ 2026-09-03 17:15 ` Eduard Zingerman
  2026-09-04  2:00 ` [PATCH bpf 1/3] bpf: zero extend the result " patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-09-03 17:15 UTC (permalink / raw)
  To: bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc

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 <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
---
 .../selftests/bpf/progs/verifier_zext.c       | 26 +++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_zext.c b/tools/testing/selftests/bpf/progs/verifier_zext.c
index 8f2362da91d6..572017fe28fb 100644
--- a/tools/testing/selftests/bpf/progs/verifier_zext.c
+++ b/tools/testing/selftests/bpf/progs/verifier_zext.c
@@ -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. */
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
  2026-09-03 17:15 ` [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics Eduard Zingerman
@ 2026-09-03 18:11   ` bot+bpf-ci
  2026-09-03 19:38     ` Eduard Zingerman
  0 siblings, 1 reply; 6+ messages in thread
From: bot+bpf-ci @ 2026-09-03 18:11 UTC (permalink / raw)
  To: eddyz87, bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, eddyz87, memxor,
	npc, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song,
	mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics
  2026-09-03 18:11   ` bot+bpf-ci
@ 2026-09-03 19:38     ` Eduard Zingerman
  0 siblings, 0 replies; 6+ messages in thread
From: Eduard Zingerman @ 2026-09-03 19:38 UTC (permalink / raw)
  To: bot+bpf-ci, bpf, ast, andrii
  Cc: daniel, martin.lau, kernel-team, yonghong.song, memxor, npc,
	martin.lau, mason, ihor.solodrai

On Thu, 2026-09-03 at 18:11 +0000, bot+bpf-ci@kernel.org wrote:
> > 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?

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

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

will respin v2, waiting for sashiko.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg
  2026-09-03 17:15 [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg Eduard Zingerman
  2026-09-03 17:15 ` [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics Eduard Zingerman
  2026-09-03 17:15 ` [PATCH bpf 3/3] selftests/bpf: check zero extension of an arena 32-bit cmpxchg Eduard Zingerman
@ 2026-09-04  2:00 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-09-04  2:00 UTC (permalink / raw)
  To: Eduard Zingerman
  Cc: bpf, ast, andrii, daniel, martin.lau, kernel-team, yonghong.song,
	memxor, npc

Hello:

This series was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:

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().
> 
> 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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-04  2:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 17:15 [PATCH bpf 1/3] bpf: zero extend the result of an arena 32-bit cmpxchg Eduard Zingerman
2026-09-03 17:15 ` [PATCH bpf 2/3] bpf: update disasm.c to print BPF_PROBE_ATOMIC as atomics Eduard Zingerman
2026-09-03 18:11   ` bot+bpf-ci
2026-09-03 19:38     ` Eduard Zingerman
2026-09-03 17:15 ` [PATCH bpf 3/3] selftests/bpf: check zero extension of an arena 32-bit cmpxchg Eduard Zingerman
2026-09-04  2:00 ` [PATCH bpf 1/3] bpf: zero extend the result " patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox