mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena
@ 2026-09-24 16:03 Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 1/3] " Puranjay Mohan
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:03 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Tejun Heo,
	David Vernet, Andrea Righi, Changwoo Min, sched-ext,
	linux-kernel

Changelog:
v1: https://lore.kernel.org/all/20260923165301.3463007-1-puranjay@kernel.org/
Changes in v2:
- Patch 1: drop the initial load in the arena lowering instead of adding a
  second exception table entry, per Alexei's review. A losing CMPXCHG
  reloads RAX from memory, so the loop converges without it and the CMPXCHG
  is left as the only insn that can fault. This removes the entry the
  verifier had not reserved, and with it the aux->num_exentries rescan in
  bpf_int_jit_compile() that v1 needed to account for it.
- Patch 1: say in the changelog that the bpf_jit_supports_insn() rejection
  is dropped, since that is what newly admits these programs, and that the
  arena form now always executes at least two locked CMPXCHGs.
- Patch 1: rename the shared "mem32 extable bug" message, which the atomic
  path can now raise, let the non-arena caller pass NULL for the fault and
  resume out-params, correct the INSN_LEN comment about where the bound is
  enforced, and drop an unrelated whitespace hunk.
- Patch 2: keep one __c11_atomic_fetch_*() per operation with the result
  discarded. v1 converted every case to the fetching form, which left the
  non-fetching lowering with no coverage at all, on every architecture
  rather than just x86.
- Patch 2: add both R0-aliased forms to the uaf test too. v1 only exercised
  them on live pages, so the BPF_REG_AX substitution never reached the
  exception table, which is the part patch 1 actually adds.
- Patch 2: order the new globals so they open fewer holes in .addr_space.1,
  and correct the comment about why arm64 stays out of the uaf test.
- Patch 3: new.

A fetching AND/OR/XOR against arena memory is rejected on x86-64:

  BPF_ATOMIC stores into R1 arena is not allowed

x86-64 has no single instruction for these, so the JIT lowers them to a
CMPXCHG loop. The loop performs two memory accesses, the load of the old
value and the CMPXCHG itself, and either can fault when the arena page
goes away. The verifier reserves one exception table entry per
instruction, so there was nowhere to record the second one and
bpf_jit_supports_insn() refused the three opcodes instead.

x86-64 is the only architecture that needs this. riscv64 has native
AMOAND/AMOOR/AMOXOR with fetch, s390 has LAN/LAO/LAX, and arm64 with LSE
has LDCLRAL/LDSETAL/LDEORAL, so all three already accept these in an
arena. arm64 without LSE rejects every arena RMW atomic and is unaffected
either way, since the CMPXCHG that such a lowering would need is not
available there in an arena either.

Patch 1 emits the loop with R12-indexed addressing and without the initial
load. A CMPXCHG that loses the comparison loads the current contents into
RAX, so the loop converges from whatever R0 already holds, and the value it
stores is computed from RAX, which by definition equalled memory whenever
the store happened. That leaves the CMPXCHG as the only insn that can
fault, so the one entry the verifier already reserved is enough and nothing
has to touch aux->num_exentries. The entry resumes past the whole loop
rather than past the faulting instruction, with the fetch destination
cleared, so a fault cannot re-enter the loop. The first CMPXCHG compares
against an unrelated value, so the arena form executes at least two locked
CMPXCHGs every time; the non-arena lowering therefore keeps its load.

Patch 2 makes the selftests cover this. The existing arena and/or/xor tests
discarded the returned value, so clang emitted the non-fetching instruction
and the fetching one was never exercised. That was deliberate:
commit 2897b1e2a2f4 ("selftests/bpf: Fix arena_atomics failure due to llvm
change") switched them to __c11_atomic_fetch_*() with memory_order_relaxed
to dodge the limitation patch 1 removes. They now check the old
value via __sync_fetch_and_*(), while keeping one __c11_atomic_fetch_*()
per operation with the result discarded so the non-fetching lowering does
not lose its only coverage. A new fetch_r0 test pins the two register
assignments the JIT special-cases, and both also go into the uaf test,
since the BPF_REG_AX substitution only reaches the exception table when
such an access faults. x86 is dropped from the uaf exclusion list.

Patch 3 fixes a sched_ext comment that justifies a hand-rolled cmpxchg loop
by naming the x86 bpf_jit_supports_insn() rejection patch 1 removes. The
loop still has to stay: LSE is compiled in unconditionally since commit
6191b25d8bd9 ("arm64: Unconditionally enable LSE support"), but the switch
is still dynamic, so an ARMv8.0 CPU runs LL/SC and the arm64 JIT keeps
rejecting arena RMW atomics there. Only the reason was stale.

Puranjay Mohan (3):
  bpf, x86: Support fetching AND/OR/XOR atomics in arena
  selftests/bpf: Test fetching AND/OR/XOR atomics in arena
  sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop

 arch/x86/net/bpf_jit_comp.c                   | 236 ++++++++++++------
 tools/sched_ext/include/scx/cid.bpf.h         |   8 +-
 .../selftests/bpf/prog_tests/arena_atomics.c  |  38 +++
 .../selftests/bpf/progs/arena_atomics.c       | 156 +++++++++---
 4 files changed, 318 insertions(+), 120 deletions(-)


base-commit: 4f3a5eae895b9995e93425a75235d8f1f3268caa
-- 
2.53.0-Meta


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

* [PATCH bpf-next v2 1/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena
  2026-09-24 16:03 [PATCH bpf-next v2 0/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena Puranjay Mohan
@ 2026-09-24 16:03 ` Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 2/3] selftests/bpf: Test " Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop Puranjay Mohan
  2 siblings, 0 replies; 8+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:03 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Tejun Heo,
	David Vernet, Andrea Righi, Changwoo Min, sched-ext,
	linux-kernel

x86-64 has no single instruction for a fetching AND/OR/XOR, so the JIT
lowers them to a CMPXCHG loop. That loop could not be used against arena
memory: it contains two memory accesses, the load of the old value and
the CMPXCHG itself, either of which can fault when the arena page goes
away, while the verifier reserves only one exception table entry per
instruction. bpf_jit_supports_insn() therefore rejected the three opcodes
and such programs failed to load with

  BPF_ATOMIC stores into R1 arena is not allowed

Emit the loop without the initial load. A CMPXCHG that loses the
comparison loads the current contents into RAX, so the loop converges
from whatever R0 already holds, and the value it stores is computed from
RAX, which by definition equalled memory whenever the store happened.
That leaves the CMPXCHG as the only insn that can fault, so one exception
table entry is enough and the count the verifier already reserved still
matches. Drop the bpf_jit_supports_insn() rejection that gated all this.

The first CMPXCHG compares against an unrelated value, so barring
coincidence it always loses and the arena form executes at least two
locked CMPXCHGs every time. A losing locked CMPXCHG is still a full
read-modify-write, so this is a real steady-state cost rather than an
occasional retry, which is why the non-arena lowering keeps its load.

The entry resumes past the whole loop rather than past the faulting
instruction alone, with the fetch destination cleared, so a fault cannot
re-enter the loop. This repurposes the INSN_LEN field of the fixup as a
resume distance rather than an instruction length; a sequence long enough
to overflow its 8 bits would first have to exceed BPF_MAX_INSN_SIZE, which
do_jit() rejects before the image is used.

The loop needs RAX for CMPXCHG and substitutes BPF_REG_AX for R0 when
either operand is R0, so add the matching reg2pt_regs[] entry:
ex_handler_bpf() now has to name that register both as the one holding
the arena address and as the one to clear on fault.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 236 ++++++++++++++++++++++++------------
 1 file changed, 160 insertions(+), 76 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 9fbef7504e51a..116d17d6e5c58 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -236,8 +236,17 @@ static const int reg2pt_regs[] = {
 	[BPF_REG_7] = offsetof(struct pt_regs, r13),
 	[BPF_REG_8] = offsetof(struct pt_regs, r14),
 	[BPF_REG_9] = offsetof(struct pt_regs, r15),
+	/* Substituted for R0 by the CMPXCHG loop lowering below. */
+	[BPF_REG_AX] = offsetof(struct pt_regs, r10),
 };
 
+static bool is_atomic_fetch_op(const struct bpf_insn *insn)
+{
+	return insn->imm == (BPF_AND | BPF_FETCH) ||
+	       insn->imm == (BPF_OR | BPF_FETCH) ||
+	       insn->imm == (BPF_XOR | BPF_FETCH);
+}
+
 /*
  * is_ereg() == true if BPF register 'reg' maps to x86-64 r8..r15
  * which need extra byte of encoding.
@@ -1679,6 +1688,74 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
 	return 0;
 }
 
+/*
+ * A fetching AND/OR/XOR can't be implemented with a single x86 insn, so do a
+ * CMPXCHG loop. @index_reg is X86_REG_R12 for an arena access or -1 otherwise,
+ * and @dst_reg/@src_reg are already substituted for R0 by the caller.
+ *
+ * For an arena access the CMPXCHG is the only insn that can fault; its address
+ * is handed back in @fault. A fault has to resume at @resume, which is past the
+ * loop and past the move that delivers the old value, but before the R0
+ * restore. Both are NULL for the non-arena case, which needs no fixup.
+ */
+static int emit_atomic_fetch_rmw(u8 **pprog, struct bpf_insn *insn, u32 dst_reg,
+				 u32 src_reg, int index_reg, u8 **fault,
+				 u8 **resume)
+{
+	bool is64 = BPF_SIZE(insn->code) == BPF_DW;
+	u8 *branch_target, *prog = *pprog;
+	int err;
+
+	branch_target = prog;
+
+	/*
+	 * Load old value. The arena case skips it and lets the loop start from
+	 * whatever R0 happens to hold: a CMPXCHG that loses the comparison
+	 * loads the current contents into RAX, so the loop converges, and the
+	 * value it stores is computed from RAX, which by definition equalled
+	 * memory whenever the store happened. That leaves the CMPXCHG as the
+	 * only insn that can fault, which is worth an extra iteration here to
+	 * keep this to a single exception table entry.
+	 */
+	if (index_reg < 0)
+		emit_ldx(&prog, BPF_SIZE(insn->code), BPF_REG_0, dst_reg, insn->off);
+
+	/*
+	 * Perform the (commutative) operation locally, put the result in
+	 * the AUX_REG.
+	 */
+	emit_mov_reg(&prog, is64, AUX_REG, BPF_REG_0);
+	maybe_emit_mod(&prog, AUX_REG, src_reg, is64);
+	EMIT2(simple_alu_opcodes[BPF_OP(insn->imm)],
+	      add_2reg(0xC0, AUX_REG, src_reg));
+
+	/* Attempt to swap in new value */
+	if (fault)
+		*fault = prog;
+	if (index_reg < 0)
+		err = emit_atomic_rmw(&prog, BPF_CMPXCHG, dst_reg, AUX_REG,
+				      insn->off, BPF_SIZE(insn->code));
+	else
+		err = emit_atomic_rmw_index(&prog, BPF_CMPXCHG, BPF_SIZE(insn->code),
+					    dst_reg, AUX_REG, index_reg, insn->off);
+	if (WARN_ON(err))
+		return err;
+
+	/* ZF tells us whether we won the race. If it's cleared we need to try again. */
+	EMIT2(X86_JNE, -(prog - branch_target) - 2);
+	/* Return the pre-modification value */
+	emit_mov_reg(&prog, is64, src_reg, BPF_REG_0);
+
+	if (resume)
+		*resume = prog;
+
+	/* Restore R0 after clobbering RAX */
+	emit_mov_reg(&prog, true, BPF_REG_0, BPF_REG_AX);
+
+	*pprog = prog;
+	return 0;
+}
+
 /*
  * Metadata encoding for exception handling in JITed code.
  *
@@ -1692,7 +1769,12 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
  * | ARENA_ACC | ARENA_WRITE | Unused | ARENA_REG | DST_REG | INSN_LEN |
  * +-----------+-------------+--------+-----------+---------+----------+
  *
- * - INSN_LEN (8 bits): Length of faulting insn (max x86 insn = 15 bytes (fits in 8 bits)).
+ * - INSN_LEN (8 bits): How far past the faulting insn to resume. That is its own length
+ *                      for a single-insn access, but the distance to the end of the whole
+ *                      sequence where one BPF insn became several, as for the CMPXCHG loop
+ *                      of a fetching AND/OR/XOR. A sequence long enough to overflow this
+ *                      field would first have to exceed BPF_MAX_INSN_SIZE, which do_jit()
+ *                      rejects with -EFAULT before the image is used.
  * - DST_REG  (8 bits): Offset of dst_reg from reg2pt_regs[] (max offset = 112 (fits in 8 bits)).
  *                      This is set to DONT_CLEAR if the insn does not read into a register.
  * - ARENA_REG (8 bits): Offset of the register that is used to calculate the
@@ -1747,6 +1829,44 @@ bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
 	return true;
 }
 
+/*
+ * Record an arena access that may fault. @fault_ip is the address of the
+ * faulting insn in the RO image, @resume_off how far past it execution has to
+ * resume: for a multi-insn lowering that is the end of the whole sequence, not
+ * the end of the one insn.
+ */
+static int emit_arena_exentry(struct bpf_prog *bpf_prog, u8 *image, u8 *rw_image,
+			      int *excnt, u8 *fault_ip, u32 resume_off,
+			      u32 fixup_reg, u32 arena_reg, bool is_write, s16 off)
+{
+	struct exception_table_entry *ex;
+	s64 delta;
+
+	if (!bpf_prog->aux->extable)
+		return 0;
+
+	if (*excnt >= bpf_prog->aux->num_exentries) {
+		pr_err("arena extable bug\n");
+		return -EFAULT;
+	}
+	ex = &bpf_prog->aux->extable[(*excnt)++];
+
+	delta = fault_ip - (u8 *)&ex->insn;
+	/* switch ex to rw buffer for writes */
+	ex = (void *)rw_image + ((void *)ex - (void *)image);
+
+	ex->insn = delta;
+	ex->data = EX_TYPE_BPF | FIELD_PREP(DATA_ARENA_OFFSET_MASK, off);
+	ex->fixup = FIELD_PREP(FIXUP_INSN_LEN_MASK, resume_off) |
+		    FIELD_PREP(FIXUP_ARENA_REG_MASK, arena_reg) |
+		    FIELD_PREP(FIXUP_REG_MASK, fixup_reg) |
+		    FIXUP_ARENA_ACCESS;
+	if (is_write)
+		ex->fixup |= FIXUP_ARENA_WRITE;
+
+	return 0;
+}
+
 static void detect_reg_usage(struct bpf_insn *insn, int insn_cnt,
 			     bool *regs_used)
 {
@@ -2624,28 +2744,8 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 			}
 populate_extable:
 			{
-				struct exception_table_entry *ex;
-				u8 *_insn = image + proglen + (start_of_ldx - temp);
 				u32 arena_reg, fixup_reg;
 				bool is_write;
-				s64 delta;
-
-				if (!bpf_prog->aux->extable)
-					break;
-
-				if (excnt >= bpf_prog->aux->num_exentries) {
-					pr_err("mem32 extable bug\n");
-					return -EFAULT;
-				}
-				ex = &bpf_prog->aux->extable[excnt++];
-
-				delta = _insn - (u8 *)&ex->insn;
-				/* switch ex to rw buffer for writes */
-				ex = (void *)rw_image + ((void *)ex - (void *)image);
-
-				ex->insn = delta;
-
-				ex->data = EX_TYPE_BPF;
 
 				/*
 				 * src_reg/dst_reg holds the address in the arena region with upper
@@ -2681,14 +2781,12 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 					is_write = true;
 				}
 
-				ex->fixup = FIELD_PREP(FIXUP_INSN_LEN_MASK, prog - start_of_ldx) |
-					    FIELD_PREP(FIXUP_ARENA_REG_MASK, arena_reg) |
-					    FIELD_PREP(FIXUP_REG_MASK, fixup_reg);
-				ex->fixup |= FIXUP_ARENA_ACCESS;
-				if (is_write)
-					ex->fixup |= FIXUP_ARENA_WRITE;
-
-				ex->data |= FIELD_PREP(DATA_ARENA_OFFSET_MASK, insn->off);
+				err = emit_arena_exentry(bpf_prog, image, rw_image, &excnt,
+							 image + proglen + (start_of_ldx - temp),
+							 prog - start_of_ldx, fixup_reg,
+							 arena_reg, is_write, insn->off);
+				if (err)
+					return err;
 			}
 			break;
 
@@ -2840,20 +2938,12 @@ 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: {
-			bool is64 = BPF_SIZE(insn->code) == BPF_DW;
 			u32 real_src_reg = src_reg;
 			u32 real_dst_reg = dst_reg;
+			bool is_atomic_fetch = is_atomic_fetch_op(insn);
 			u8 *old_prog;
-			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.
-				 */
 
+			if (is_atomic_fetch) {
 				/* Will need RAX as a CMPXCHG operand so save R0 */
 				old_prog = prog;
 				emit_mov_reg(&prog, true, BPF_REG_AX, BPF_REG_0);
@@ -2874,34 +2964,11 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 				}
 			}
 			if (is_atomic_fetch) {
-				u8 *branch_target = prog;
-				/* Load old value */
-				emit_ldx(&prog, BPF_SIZE(insn->code),
-					 BPF_REG_0, real_dst_reg, insn->off);
-				/*
-				 * Perform the (commutative) operation locally,
-				 * put the result in the AUX_REG.
-				 */
-				emit_mov_reg(&prog, is64, AUX_REG, BPF_REG_0);
-				maybe_emit_mod(&prog, AUX_REG, real_src_reg, is64);
-				EMIT2(simple_alu_opcodes[BPF_OP(insn->imm)],
-				      add_2reg(0xC0, AUX_REG, real_src_reg));
-				/* Attempt to swap in new value */
-				err = emit_atomic_rmw(&prog, BPF_CMPXCHG,
-						      real_dst_reg, AUX_REG,
-						      insn->off,
-						      BPF_SIZE(insn->code));
-				if (WARN_ON(err))
+				err = emit_atomic_fetch_rmw(&prog, insn, real_dst_reg,
+							    real_src_reg, -1, NULL,
+							    NULL);
+				if (err)
 					return err;
-				/*
-				 * ZF tells us whether we won the race. If it's
-				 * cleared we need to try again.
-				 */
-				EMIT2(X86_JNE, -(prog - branch_target) - 2);
-				/* Return the pre-modification value */
-				emit_mov_reg(&prog, is64, real_src_reg, BPF_REG_0);
-				/* Restore R0 after clobbering RAX */
-				emit_mov_reg(&prog, true, BPF_REG_0, BPF_REG_AX);
 				break;
 			}
 
@@ -2925,6 +2992,33 @@ static int do_jit(struct bpf_verifier_env *env, struct bpf_prog *bpf_prog, int *
 			fallthrough;
 		case BPF_STX | BPF_PROBE_ATOMIC | BPF_W:
 		case BPF_STX | BPF_PROBE_ATOMIC | BPF_DW:
+			if (is_atomic_fetch_op(insn)) {
+				u32 real_src_reg = src_reg, real_dst_reg = dst_reg;
+				u8 *fault, *resume;
+
+				/* 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;
+
+				err = emit_atomic_fetch_rmw(&prog, insn, real_dst_reg,
+							    real_src_reg, X86_REG_R12,
+							    &fault, &resume);
+				if (err)
+					return err;
+
+				err = emit_arena_exentry(bpf_prog, image, rw_image, &excnt,
+							 image + proglen + (fault - temp),
+							 resume - fault,
+							 reg2pt_regs[real_src_reg],
+							 reg2pt_regs[real_dst_reg],
+							 true, insn->off);
+				if (err)
+					return err;
+				break;
+			}
 			start_of_ldx = prog;
 
 			if (bpf_atomic_is_load_store(insn))
@@ -4645,16 +4739,6 @@ bool bpf_jit_supports_arena(void)
 
 bool bpf_jit_supports_insn(struct bpf_insn *insn, bool in_arena)
 {
-	if (!in_arena)
-		return true;
-	switch (insn->code) {
-	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))
-			return false;
-	}
 	return true;
 }
 
-- 
2.53.0-Meta


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

* [PATCH bpf-next v2 2/3] selftests/bpf: Test fetching AND/OR/XOR atomics in arena
  2026-09-24 16:03 [PATCH bpf-next v2 0/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 1/3] " Puranjay Mohan
@ 2026-09-24 16:03 ` Puranjay Mohan
  2026-09-24 16:29   ` Alexei Starovoitov
  2026-09-24 16:55   ` bot+bpf-ci
  2026-09-24 16:03 ` [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop Puranjay Mohan
  2 siblings, 2 replies; 8+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:03 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Tejun Heo,
	David Vernet, Andrea Righi, Changwoo Min, sched-ext,
	linux-kernel

The arena and/or/xor tests never exercised the fetching form of the
instruction. Commit 2897b1e2a2f4 ("selftests/bpf: Fix arena_atomics
failure due to llvm change") deliberately switched them to
__c11_atomic_fetch_*() with memory_order_relaxed, which clang lowers to a
non-fetching locked instruction when the result is unused, because
x86-64 could not JIT the fetching one against an arena.

It can now, so check the returned old value with __sync_fetch_and_*().
Keep one __c11_atomic_fetch_*() per operation with the result discarded:
that spelling is what selects the non-fetching insn, which would
otherwise lose its only coverage, on every architecture rather than just
x86. Compiling the prog before and after confirms the split: 6 non-
fetching and no fetching arena and/or/xor before, 3 and 22 after.

Add a fetch_r0 test for the two register assignments the x86 JIT has to
special-case, where the operand and where the arena pointer is R0 and
BPF_REG_AX is substituted for it. Clang picks its own registers and will
not reliably produce either, so spell the instructions out. Add both
forms to the uaf test as well, since the substitution only reaches the
exception table when such an access faults.

Finally drop x86 from the exclusion list of the uaf test, which is what
covers the fault path through the new exception table entry.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 .../selftests/bpf/prog_tests/arena_atomics.c  |  38 +++++
 .../selftests/bpf/progs/arena_atomics.c       | 156 +++++++++++++-----
 2 files changed, 154 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
index 1ad5d03d07adb..42061e70e1a34 100644
--- a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
+++ b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
@@ -68,6 +68,11 @@ static void test_and(struct arena_atomics *skel)
 
 	ASSERT_EQ(skel->arena->and64_value, 0x010ull << 32, "and64_value");
 	ASSERT_EQ(skel->arena->and32_value, 0x010, "and32_value");
+
+	ASSERT_EQ(skel->arena->and64_result, 0x110ull << 32, "and64_result");
+	ASSERT_EQ(skel->arena->and32_result, 0x110, "and32_result");
+
+	ASSERT_EQ(skel->arena->and64_noreturn_value, 0x010ull << 32, "and64_noreturn_value");
 }
 
 static void test_or(struct arena_atomics *skel)
@@ -85,6 +90,11 @@ static void test_or(struct arena_atomics *skel)
 
 	ASSERT_EQ(skel->arena->or64_value, 0x111ull << 32, "or64_value");
 	ASSERT_EQ(skel->arena->or32_value, 0x111, "or32_value");
+
+	ASSERT_EQ(skel->arena->or64_result, 0x110ull << 32, "or64_result");
+	ASSERT_EQ(skel->arena->or32_result, 0x110, "or32_result");
+
+	ASSERT_EQ(skel->arena->or64_noreturn_value, 0x111ull << 32, "or64_noreturn_value");
 }
 
 static void test_xor(struct arena_atomics *skel)
@@ -102,6 +112,11 @@ static void test_xor(struct arena_atomics *skel)
 
 	ASSERT_EQ(skel->arena->xor64_value, 0x101ull << 32, "xor64_value");
 	ASSERT_EQ(skel->arena->xor32_value, 0x101, "xor32_value");
+
+	ASSERT_EQ(skel->arena->xor64_result, 0x110ull << 32, "xor64_result");
+	ASSERT_EQ(skel->arena->xor32_result, 0x110, "xor32_result");
+
+	ASSERT_EQ(skel->arena->xor64_noreturn_value, 0x101ull << 32, "xor64_noreturn_value");
 }
 
 static void test_cmpxchg(struct arena_atomics *skel)
@@ -146,6 +161,27 @@ static void test_xchg(struct arena_atomics *skel)
 	ASSERT_EQ(skel->arena->xchg32_result, 1, "xchg32_result");
 }
 
+static void test_fetch_r0(struct arena_atomics *skel)
+{
+	LIBBPF_OPTS(bpf_test_run_opts, topts);
+	int err, prog_fd;
+
+	/* No need to attach it, just run it directly */
+	prog_fd = bpf_program__fd(skel->progs.fetch_r0);
+	err = bpf_prog_test_run_opts(prog_fd, &topts);
+	if (!ASSERT_OK(err, "test_run_opts err"))
+		return;
+	if (!ASSERT_OK(topts.retval, "test_run_opts retval"))
+		return;
+
+	ASSERT_EQ(skel->arena->fetch_src_r0_value, 0x111, "fetch_src_r0_value");
+	ASSERT_EQ(skel->arena->fetch_src_r0_result, 0x110, "fetch_src_r0_result");
+
+	ASSERT_EQ(skel->arena->fetch_dst_r0_value, 0x111, "fetch_dst_r0_value");
+	ASSERT_EQ(skel->arena->fetch_dst_r0_result, 0x110, "fetch_dst_r0_result");
+	ASSERT_EQ(skel->arena->fetch_dst_r0_readback, 0x111, "fetch_dst_r0_readback");
+}
+
 static void test_uaf(struct arena_atomics *skel)
 {
 	LIBBPF_OPTS(bpf_test_run_opts, topts);
@@ -256,6 +292,8 @@ void serial_test_arena_atomics(void)
 		test_cmpxchg(skel);
 	if (test__start_subtest("xchg"))
 		test_xchg(skel);
+	if (test__start_subtest("fetch_r0"))
+		test_fetch_r0(skel);
 	if (test__start_subtest("uaf"))
 		test_uaf(skel);
 	if (test__start_subtest("load_acquire"))
diff --git a/tools/testing/selftests/bpf/progs/arena_atomics.c b/tools/testing/selftests/bpf/progs/arena_atomics.c
index 73bc2b835f3fe..9a3eee526cc9e 100644
--- a/tools/testing/selftests/bpf/progs/arena_atomics.c
+++ b/tools/testing/selftests/bpf/progs/arena_atomics.c
@@ -91,13 +91,11 @@ int sub(const void *ctx)
 	return 0;
 }
 
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-_Atomic __u64 __arena_global and64_value = (0x110ull << 32);
-_Atomic __u32 __arena_global and32_value = 0x110;
-#else
 __u64 __arena_global and64_value = (0x110ull << 32);
+__u64 __arena_global and64_result = 0;
+_Atomic __u64 __arena_global and64_noreturn_value = (0x110ull << 32);
 __u32 __arena_global and32_value = 0x110;
-#endif
+__u32 __arena_global and32_result = 0;
 
 SEC("raw_tp/sys_enter")
 int and(const void *ctx)
@@ -105,25 +103,20 @@ int and(const void *ctx)
 	if (pid != (bpf_get_current_pid_tgid() >> 32))
 		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-	__c11_atomic_fetch_and(&and64_value, 0x011ull << 32, memory_order_relaxed);
-	__c11_atomic_fetch_and(&and32_value, 0x011, memory_order_relaxed);
-#else
-	__sync_fetch_and_and(&and64_value, 0x011ull << 32);
-	__sync_fetch_and_and(&and32_value, 0x011);
-#endif
+	and64_result = __sync_fetch_and_and(&and64_value, 0x011ull << 32);
+	and32_result = __sync_fetch_and_and(&and32_value, 0x011);
+	/* Discarding the result is what selects the non-fetching insn. */
+	__c11_atomic_fetch_and(&and64_noreturn_value, 0x011ull << 32, memory_order_relaxed);
 #endif
 
 	return 0;
 }
 
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-_Atomic __u32 __arena_global or32_value = 0x110;
-_Atomic __u64 __arena_global or64_value = (0x110ull << 32);
-#else
-__u32 __arena_global or32_value = 0x110;
 __u64 __arena_global or64_value = (0x110ull << 32);
-#endif
+__u64 __arena_global or64_result = 0;
+_Atomic __u64 __arena_global or64_noreturn_value = (0x110ull << 32);
+__u32 __arena_global or32_value = 0x110;
+__u32 __arena_global or32_result = 0;
 
 SEC("raw_tp/sys_enter")
 int or(const void *ctx)
@@ -131,25 +124,20 @@ int or(const void *ctx)
 	if (pid != (bpf_get_current_pid_tgid() >> 32))
 		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-	__c11_atomic_fetch_or(&or64_value, 0x011ull << 32, memory_order_relaxed);
-	__c11_atomic_fetch_or(&or32_value, 0x011, memory_order_relaxed);
-#else
-	__sync_fetch_and_or(&or64_value, 0x011ull << 32);
-	__sync_fetch_and_or(&or32_value, 0x011);
-#endif
+	or64_result = __sync_fetch_and_or(&or64_value, 0x011ull << 32);
+	or32_result = __sync_fetch_and_or(&or32_value, 0x011);
+	/* Discarding the result is what selects the non-fetching insn. */
+	__c11_atomic_fetch_or(&or64_noreturn_value, 0x011ull << 32, memory_order_relaxed);
 #endif
 
 	return 0;
 }
 
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-_Atomic __u64 __arena_global xor64_value = (0x110ull << 32);
-_Atomic __u32 __arena_global xor32_value = 0x110;
-#else
 __u64 __arena_global xor64_value = (0x110ull << 32);
+__u64 __arena_global xor64_result = 0;
+_Atomic __u64 __arena_global xor64_noreturn_value = (0x110ull << 32);
 __u32 __arena_global xor32_value = 0x110;
-#endif
+__u32 __arena_global xor32_result = 0;
 
 SEC("raw_tp/sys_enter")
 int xor(const void *ctx)
@@ -157,13 +145,10 @@ int xor(const void *ctx)
 	if (pid != (bpf_get_current_pid_tgid() >> 32))
 		return 0;
 #ifdef ENABLE_ATOMICS_TESTS
-#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
-	__c11_atomic_fetch_xor(&xor64_value, 0x011ull << 32, memory_order_relaxed);
-	__c11_atomic_fetch_xor(&xor32_value, 0x011, memory_order_relaxed);
-#else
-	__sync_fetch_and_xor(&xor64_value, 0x011ull << 32);
-	__sync_fetch_and_xor(&xor32_value, 0x011);
-#endif
+	xor64_result = __sync_fetch_and_xor(&xor64_value, 0x011ull << 32);
+	xor32_result = __sync_fetch_and_xor(&xor32_value, 0x011);
+	/* Discarding the result is what selects the non-fetching insn. */
+	__c11_atomic_fetch_xor(&xor64_noreturn_value, 0x011ull << 32, memory_order_relaxed);
 #endif
 
 	return 0;
@@ -213,6 +198,64 @@ int xchg(const void *ctx)
 	return 0;
 }
 
+__u64 __arena_global fetch_src_r0_value = 0x110;
+__u64 __arena_global fetch_src_r0_result = 0;
+__u64 __arena_global fetch_dst_r0_value = 0x110;
+__u64 __arena_global fetch_dst_r0_result = 0;
+__u64 __arena_global fetch_dst_r0_readback = 0;
+
+/*
+ * A fetching OR with the operand in r0, and one with the arena pointer in r0.
+ * The x86 JIT needs RAX for its CMPXCHG loop and substitutes BPF_REG_AX for
+ * whichever of the two is r0, so both have to keep working. Hand-written
+ * because clang picks its own registers and will not reliably emit either.
+ */
+SEC("raw_tp/sys_enter")
+int fetch_r0(const void *ctx)
+{
+	if (pid != (bpf_get_current_pid_tgid() >> 32))
+		return 0;
+#if defined(ENABLE_ATOMICS_TESTS) && defined(__BPF_FEATURE_ADDR_SPACE_CAST)
+	asm volatile (
+	"r1 = %[fetch_src_r0_value] ll;"
+	"r1 = addr_space_cast(r1, 0x0, 0x1);"
+	"r0 = 0x011;"
+	".8byte %[fetch_src_r0_insn];"
+	"r2 = %[fetch_src_r0_result] ll;"
+	"r2 = addr_space_cast(r2, 0x0, 0x1);"
+	"*(u64 *)(r2 + 0) = r0;"
+	:
+	: __imm_addr(fetch_src_r0_value),
+	  __imm_insn(fetch_src_r0_insn,
+		     BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_1, BPF_REG_0, 0)),
+	  __imm_addr(fetch_src_r0_result)
+	: __clobber_all);
+
+	asm volatile (
+	"r0 = %[fetch_dst_r0_value] ll;"
+	"r0 = addr_space_cast(r0, 0x0, 0x1);"
+	"r1 = 0x011;"
+	".8byte %[fetch_dst_r0_insn];"
+	"r2 = %[fetch_dst_r0_result] ll;"
+	"r2 = addr_space_cast(r2, 0x0, 0x1);"
+	"*(u64 *)(r2 + 0) = r1;"
+	/* r0 is only read by the atomic, so it must still be the pointer. */
+	"r3 = *(u64 *)(r0 + 0);"
+	"r2 = %[fetch_dst_r0_readback] ll;"
+	"r2 = addr_space_cast(r2, 0x0, 0x1);"
+	"*(u64 *)(r2 + 0) = r3;"
+	:
+	: __imm_addr(fetch_dst_r0_value),
+	  __imm_insn(fetch_dst_r0_insn,
+		     BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_0, BPF_REG_1, 0)),
+	  __imm_addr(fetch_dst_r0_result),
+	  __imm_addr(fetch_dst_r0_readback)
+	: __clobber_all);
+#endif
+
+	return 0;
+}
+
 __u64 __arena_global uaf_sink;
 volatile __u64 __arena_global uaf_recovery_fails;
 
@@ -221,15 +264,20 @@ int uaf(const void *ctx)
 {
 	if (pid != (bpf_get_current_pid_tgid() >> 32))
 		return 0;
-#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64) && \
-    !defined(__TARGET_ARCH_x86)
+/*
+ * arm64 stays excluded as it has been since this test was added: its JIT only
+ * accepts arena RMW atomics when LSE is present, which is a run-time property.
+ * The rest of the file shares that dependency without a guard, so on a non-LSE
+ * arm64 the whole skeleton already fails to load.
+ */
+#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64)
 	__u32 __arena *page32;
 	__u64 __arena *page64;
 	void __arena *page;
 
 	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
 	bpf_arena_free_pages(&arena, page, 1);
-	uaf_recovery_fails = 24;
+	uaf_recovery_fails = 26;
 
 	page32 = (__u32 __arena *)page;
 	uaf_sink += __sync_fetch_and_add(page32, 1);
@@ -282,6 +330,34 @@ int uaf(const void *ctx)
 	uaf_recovery_fails -= 1;
 	uaf_sink += __sync_lock_test_and_set(page64, 1);
 	uaf_recovery_fails -= 1;
+
+	/*
+	 * The x86 JIT needs RAX for the CMPXCHG loop it lowers a fetching
+	 * AND/OR/XOR into, and substitutes BPF_REG_AX for r0 when r0 is either
+	 * operand. Only then does the fault fixup have to name that register,
+	 * so spell both forms out: clang picks its own and never produces them.
+	 */
+	asm volatile (
+	"r0 = %[page];"
+	"r0 = addr_space_cast(r0, 0x0, 0x1);"
+	"r1 = 1;"
+	".8byte %[fetch_dst_r0];"
+	: : [page]"r"(page64),
+	    __imm_insn(fetch_dst_r0,
+		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_0, BPF_REG_1, 0))
+	: "r0", "r1", "memory");
+	uaf_recovery_fails -= 1;
+
+	asm volatile (
+	"r1 = %[page];"
+	"r1 = addr_space_cast(r1, 0x0, 0x1);"
+	"r0 = 1;"
+	".8byte %[fetch_src_r0];"
+	: : [page]"r"(page64),
+	    __imm_insn(fetch_src_r0,
+		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_1, BPF_REG_0, 0))
+	: "r0", "r1", "memory");
+	uaf_recovery_fails -= 1;
 #endif
 
 	return 0;
-- 
2.53.0-Meta


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

* [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop
  2026-09-24 16:03 [PATCH bpf-next v2 0/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 1/3] " Puranjay Mohan
  2026-09-24 16:03 ` [PATCH bpf-next v2 2/3] selftests/bpf: Test " Puranjay Mohan
@ 2026-09-24 16:03 ` Puranjay Mohan
  2026-09-24 16:30   ` Alexei Starovoitov
  2 siblings, 1 reply; 8+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:03 UTC (permalink / raw)
  To: bpf
  Cc: Puranjay Mohan, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Song Liu, Yonghong Song, Tejun Heo,
	David Vernet, Andrea Righi, Changwoo Min, sched-ext,
	linux-kernel

cid.bpf.h explains its hand-rolled cmpxchg loop by saying the x86 BPF JIT
rejects BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on arena pointers, and
names bpf_jit_supports_insn() as the place that does it. That function no
longer rejects anything, so the comment sends the reader to code that
contradicts it.

The loop itself still has to stay: arm64 without LSE rejects every arena
read-modify-write atomic. Reword the comment to say that instead.

Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
---
 tools/sched_ext/include/scx/cid.bpf.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/sched_ext/include/scx/cid.bpf.h b/tools/sched_ext/include/scx/cid.bpf.h
index 69fb4e97bc771..41060dcf5f5f2 100644
--- a/tools/sched_ext/include/scx/cid.bpf.h
+++ b/tools/sched_ext/include/scx/cid.bpf.h
@@ -130,10 +130,10 @@ static __always_inline bool cmask_test(u32 cid, const struct scx_cmask __arena *
 }
 
 /*
- * x86 BPF JIT rejects BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on arena
- * pointers (see bpf_jit_supports_insn() in arch/x86/net/bpf_jit_comp.c). Only
- * BPF_CMPXCHG / BPF_XCHG / BPF_ADD with FETCH are allowed. Implement
- * test_and_{set,clear} and the atomic set/clear via a cmpxchg loop.
+ * Not every BPF JIT accepts BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on
+ * arena pointers: arm64 without LSE rejects every arena read-modify-write
+ * atomic. Implement test_and_{set,clear} and the atomic set/clear via a
+ * cmpxchg loop so this works everywhere.
  *
  * CMASK_CAS_TRIES is sized so exhausting it means seconds of real spinning
  * on one word - past any plausible contention. Abort hard.
-- 
2.53.0-Meta


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

* Re: [PATCH bpf-next v2 2/3] selftests/bpf: Test fetching AND/OR/XOR atomics in arena
  2026-09-24 16:03 ` [PATCH bpf-next v2 2/3] selftests/bpf: Test " Puranjay Mohan
@ 2026-09-24 16:29   ` Alexei Starovoitov
  2026-09-24 16:55   ` bot+bpf-ci
  1 sibling, 0 replies; 8+ messages in thread
From: Alexei Starovoitov @ 2026-09-24 16:29 UTC (permalink / raw)
  To: Puranjay Mohan, bpf
  Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
	Yonghong Song, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min, sched-ext, linux-kernel

On Thu, Sep 24, 2026 at 09:03 AM Puranjay Mohan <puranjay@kernel.org> wrote:
> -#ifdef __BPF_FEATURE_ATOMIC_MEM_ORDERING
> -	__c11_atomic_fetch_and(&and64_value, 0x011ull << 32, memory_order_relaxed);
> -	__c11_atomic_fetch_and(&and32_value, 0x011, memory_order_relaxed);
> -#else
> -	__sync_fetch_and_and(&and64_value, 0x011ull << 32);
> -	__sync_fetch_and_and(&and32_value, 0x011);
> -#endif
> +	and64_result = __sync_fetch_and_and(&and64_value, 0x011ull << 32);
> +	and32_result = __sync_fetch_and_and(&and32_value, 0x011);
> +	/* Discarding the result is what selects the non-fetching insn. */
> +	__c11_atomic_fetch_and(&and64_noreturn_value, 0x011ull << 32, memory_order_relaxed);

The 32-bit non-fetching and/or/xor in arena are not tested anymore.
Pls keep them too.

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

* Re: [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop
  2026-09-24 16:03 ` [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop Puranjay Mohan
@ 2026-09-24 16:30   ` Alexei Starovoitov
  2026-09-24 16:33     ` Puranjay Mohan
  0 siblings, 1 reply; 8+ messages in thread
From: Alexei Starovoitov @ 2026-09-24 16:30 UTC (permalink / raw)
  To: Puranjay Mohan, bpf
  Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
	Yonghong Song, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min, sched-ext, linux-kernel

On Thu, Sep 24, 2026 at 09:03 AM Puranjay Mohan <puranjay@kernel.org> wrote:
> - * x86 BPF JIT rejects BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on arena
> - * pointers (see bpf_jit_supports_insn() in arch/x86/net/bpf_jit_comp.c). Only
> - * BPF_CMPXCHG / BPF_XCHG / BPF_ADD with FETCH are allowed. Implement
> - * test_and_{set,clear} and the atomic set/clear via a cmpxchg loop.
> + * Not every BPF JIT accepts BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on
> + * arena pointers: arm64 without LSE rejects every arena read-modify-write
> + * atomic. Implement test_and_{set,clear} and the atomic set/clear via a
> + * cmpxchg loop so this works everywhere.

arm64 without LSE rejects BPF_CMPXCHG in arena too.
bpf_jit_supports_insn() there returns false for everything that
is not load_acq/store_rel, so the loop doesn't load on such cpu either.
riscv without Zacas is the other way around. It rejects arena cmpxchg
and accepts fetching and/or.
So the loop doesn't make it work everywhere.

After patch 1 can cmask_set/clear/test_and_set/test_and_clear
be __sync_fetch_and_or/and and CMASK_CAS_TRIES go away ?

pw-bot: cr

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

* Re: [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop
  2026-09-24 16:30   ` Alexei Starovoitov
@ 2026-09-24 16:33     ` Puranjay Mohan
  0 siblings, 0 replies; 8+ messages in thread
From: Puranjay Mohan @ 2026-09-24 16:33 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Song Liu,
	Yonghong Song, Tejun Heo, David Vernet, Andrea Righi,
	Changwoo Min, sched-ext, linux-kernel

On Thu, Sep 24, 2026 at 5:30 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Thu, Sep 24, 2026 at 09:03 AM Puranjay Mohan <puranjay@kernel.org> wrote:
> > - * x86 BPF JIT rejects BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on arena
> > - * pointers (see bpf_jit_supports_insn() in arch/x86/net/bpf_jit_comp.c). Only
> > - * BPF_CMPXCHG / BPF_XCHG / BPF_ADD with FETCH are allowed. Implement
> > - * test_and_{set,clear} and the atomic set/clear via a cmpxchg loop.
> > + * Not every BPF JIT accepts BPF_OR | BPF_FETCH and BPF_AND | BPF_FETCH on
> > + * arena pointers: arm64 without LSE rejects every arena read-modify-write
> > + * atomic. Implement test_and_{set,clear} and the atomic set/clear via a
> > + * cmpxchg loop so this works everywhere.
>
> arm64 without LSE rejects BPF_CMPXCHG in arena too.
> bpf_jit_supports_insn() there returns false for everything that
> is not load_acq/store_rel, so the loop doesn't load on such cpu either.
> riscv without Zacas is the other way around. It rejects arena cmpxchg
> and accepts fetching and/or.
> So the loop doesn't make it work everywhere.
>
> After patch 1 can cmask_set/clear/test_and_set/test_and_clear
> be __sync_fetch_and_or/and and CMASK_CAS_TRIES go away ?

Yes, I thought of doing that as a follow-up

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

* Re: [PATCH bpf-next v2 2/3] selftests/bpf: Test fetching AND/OR/XOR atomics in arena
  2026-09-24 16:03 ` [PATCH bpf-next v2 2/3] selftests/bpf: Test " Puranjay Mohan
  2026-09-24 16:29   ` Alexei Starovoitov
@ 2026-09-24 16:55   ` bot+bpf-ci
  1 sibling, 0 replies; 8+ messages in thread
From: bot+bpf-ci @ 2026-09-24 16:55 UTC (permalink / raw)
  To: puranjay, bpf
  Cc: puranjay, ast, daniel, andrii, martin.lau, eddyz87, memxor, song,
	yonghong.song, tj, void, arighi, changwoo, sched-ext,
	linux-kernel, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, mason, ihor.solodrai

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

> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
> index 1ad5d03d07ad..42061e70e1a3 100644
> --- a/tools/testing/selftests/bpf/prog_tests/arena_atomics.c
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_atomics.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/arena_atomics.c b/tools/testing/selftests/bpf/progs/arena_atomics.c
> index 73bc2b835f3f..9a3eee526cc9 100644
> --- a/tools/testing/selftests/bpf/progs/arena_atomics.c
> +++ b/tools/testing/selftests/bpf/progs/arena_atomics.c

[ ... ]

> @@ -213,6 +198,64 @@ int xchg(const void *ctx)
>  	return 0;
>  }
>
> +__u64 __arena_global fetch_src_r0_value = 0x110;
> +__u64 __arena_global fetch_src_r0_result = 0;
> +__u64 __arena_global fetch_dst_r0_value = 0x110;
> +__u64 __arena_global fetch_dst_r0_result = 0;
> +__u64 __arena_global fetch_dst_r0_readback = 0;
> +
> +/*
> + * A fetching OR with the operand in r0, and one with the arena pointer in r0.
> + * The x86 JIT needs RAX for its CMPXCHG loop and substitutes BPF_REG_AX for
> + * whichever of the two is r0, so both have to keep working. Hand-written
> + * because clang picks its own registers and will not reliably emit either.
> + */
> +SEC("raw_tp/sys_enter")
> +int fetch_r0(const void *ctx)
> +{
> +	if (pid != (bpf_get_current_pid_tgid() >> 32))
> +		return 0;
> +#if defined(ENABLE_ATOMICS_TESTS) && defined(__BPF_FEATURE_ADDR_SPACE_CAST)
> +	asm volatile (
> +	"r1 = %[fetch_src_r0_value] ll;"
> +	"r1 = addr_space_cast(r1, 0x0, 0x1);"

[ ... ]

> @@ -221,15 +264,20 @@ int uaf(const void *ctx)
>  {
>  	if (pid != (bpf_get_current_pid_tgid() >> 32))
>  		return 0;
> -#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64) && \
> -    !defined(__TARGET_ARCH_x86)
> +/*
> + * arm64 stays excluded as it has been since this test was added: its JIT only
> + * accepts arena RMW atomics when LSE is present, which is a run-time property.
> + * The rest of the file shares that dependency without a guard, so on a non-LSE
> + * arm64 the whole skeleton already fails to load.
> + */
> +#if defined(ENABLE_ATOMICS_TESTS) && !defined(__TARGET_ARCH_arm64)
>  	__u32 __arena *page32;
>  	__u64 __arena *page64;
>  	void __arena *page;
>
>  	page = bpf_arena_alloc_pages(&arena, NULL, 1, NUMA_NO_NODE, 0);
>  	bpf_arena_free_pages(&arena, page, 1);
> -	uaf_recovery_fails = 24;
> +	uaf_recovery_fails = 26;

[ ... ]

> +
> +	/*
> +	 * The x86 JIT needs RAX for the CMPXCHG loop it lowers a fetching
> +	 * AND/OR/XOR into, and substitutes BPF_REG_AX for r0 when r0 is either
> +	 * operand. Only then does the fault fixup have to name that register,
> +	 * so spell both forms out: clang picks its own and never produces them.
> +	 */
> +	asm volatile (
> +	"r0 = %[page];"
> +	"r0 = addr_space_cast(r0, 0x0, 0x1);"
> +	"r1 = 1;"
> +	".8byte %[fetch_dst_r0];"
> +	: : [page]"r"(page64),
> +	    __imm_insn(fetch_dst_r0,
> +		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_0, BPF_REG_1, 0))
> +	: "r0", "r1", "memory");
> +	uaf_recovery_fails -= 1;
> +
> +	asm volatile (
> +	"r1 = %[page];"
> +	"r1 = addr_space_cast(r1, 0x0, 0x1);"
> +	"r0 = 1;"
> +	".8byte %[fetch_src_r0];"
> +	: : [page]"r"(page64),
> +	    __imm_insn(fetch_src_r0,
> +		       BPF_ATOMIC_OP(BPF_DW, BPF_OR | BPF_FETCH, BPF_REG_1, BPF_REG_0, 0))
> +	: "r0", "r1", "memory");
> +	uaf_recovery_fails -= 1;
>  #endif

Would this build with clang 18 or earlier?  Both new asm blocks in uaf()
use the textual addr_space_cast syntax, but uaf() is only guarded by
ENABLE_ATOMICS_TESTS and !__TARGET_ARCH_arm64.  It does not check for
__BPF_FEATURE_ADDR_SPACE_CAST.

Every other use of textual addr_space_cast in this file is under that
guard: fetch_r0() above at the new code, and load_acquire/store_release
elsewhere.  The addr_space_cast asm syntax and __BPF_FEATURE_ADDR_SPACE_CAST
both arrived in LLVM 19, so with clang 18 the integrated assembler would
reject the unknown mnemonic and arena_atomics.bpf.o would fail to build.

Before this patch uaf() was plain C and was compiled out on x86, so the
asm never reached older clang there.


---
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/36026297900

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

end of thread, other threads:[~2026-09-24 16:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-24 16:03 [PATCH bpf-next v2 0/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena Puranjay Mohan
2026-09-24 16:03 ` [PATCH bpf-next v2 1/3] " Puranjay Mohan
2026-09-24 16:03 ` [PATCH bpf-next v2 2/3] selftests/bpf: Test " Puranjay Mohan
2026-09-24 16:29   ` Alexei Starovoitov
2026-09-24 16:55   ` bot+bpf-ci
2026-09-24 16:03 ` [PATCH bpf-next v2 3/3] sched_ext: Stop citing the x86 JIT for the cmask cmpxchg loop Puranjay Mohan
2026-09-24 16:30   ` Alexei Starovoitov
2026-09-24 16:33     ` Puranjay Mohan

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

all inboxes | Powered by JetHome®