[PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[Daniel Borkmann]

check_atomic_rmw() open codes the mapping from a BPF_ATOMIC to the register
it reads the old value into, the BPF_STX case of insn_def_regno() open codes
the very same mapping a second time, and BPF JITs need it as well to know
which register a faulting BPF_PROBE_ATOMIC has to clear. Having the
derivations sit in different files is how the JITs came to disagree with
the verifier in the first place. Add a small helper so that all of them can
share it. No functional change. The BPF_LOAD_ACQ case is there for the JITs,
which do walk all instruction classes.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 v1 -> v2:
   - also convert insn_def_regno (Eduard, sashiko)

 include/linux/filter.h | 24 ++++++++++++++++++++++++
 kernel/bpf/fixups.c    | 11 +----------
 kernel/bpf/verifier.c  | 17 ++++++-----------
 3 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 4edba8182db1..15d83684c6e9 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
 	       insn->imm == BPF_LOAD_ACQ;
 }
 
+/*
+ * Given an instruction @insn, return the number of the BPF register that a
+ * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is
+ * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when
+ * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to
+ * be a BPF_ATOMIC here.
+ */
+static inline int bpf_atomic_load_reg(const struct bpf_insn *insn)
+{
+	if (BPF_CLASS(insn->code) != BPF_STX ||
+	    (BPF_MODE(insn->code) != BPF_ATOMIC &&
+	     BPF_MODE(insn->code) != BPF_PROBE_ATOMIC))
+		return -1;
+
+	switch (insn->imm) {
+	case BPF_LOAD_ACQ:
+		return insn->dst_reg;
+	case BPF_CMPXCHG:
+		return BPF_REG_0;
+	default:
+		return (insn->imm & BPF_FETCH) ? insn->src_reg : -1;
+	}
+}
+
 /* Memory store, *(uint *) (dst_reg + off16) = imm32 */
 
 #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index 661e2d13a604..c4bd70befbb5 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -49,16 +49,7 @@ static int insn_def_regno(const struct bpf_insn *insn)
 	case BPF_ST:
 		return -1;
 	case BPF_STX:
-		if (BPF_MODE(insn->code) == BPF_ATOMIC ||
-		    BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
-			if (insn->imm == BPF_CMPXCHG)
-				return BPF_REG_0;
-			else if (insn->imm == BPF_LOAD_ACQ)
-				return insn->dst_reg;
-			else if (insn->imm & BPF_FETCH)
-				return insn->src_reg;
-		}
-		return -1;
+		return bpf_atomic_load_reg(insn);
 	default:
 		return insn->dst_reg;
 	}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index add3affc5703..73a2e8bb1782 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6485,21 +6485,16 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	if (insn->imm & BPF_FETCH) {
-		if (insn->imm == BPF_CMPXCHG)
-			load_reg = BPF_REG_0;
-		else
-			load_reg = insn->src_reg;
-
+	/*
+	 * A negative load_reg means that this instruction accesses a memory
+	 * location but doesn't actually load it into a register.
+	 */
+	load_reg = bpf_atomic_load_reg(insn);
+	if (load_reg >= 0) {
 		/* check and record load of old value */
 		err = check_reg_arg(env, load_reg, DST_OP);
 		if (err)
 			return err;
-	} else {
-		/* This instruction accesses a memory location but doesn't
-		 * actually load it into a register.
-		 */
-		load_reg = -1;
 	}
 
 	dst_reg = cur_regs(env) + insn->dst_reg;
-- 
2.43.0

[PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic

[Daniel Borkmann]

A RMW atomic on an arena pointer is converted to BPF_PROBE_ATOMIC and
gets an exception table entry, but that entry records no destination
register to clear unless the instruction is a load-acquire today. That
is right for a plain BPF_{ADD,AND,OR,XOR}, which only writes memory,
but an RMW carrying BPF_FETCH also reads the old value into a register:
src_reg for BPF_{ADD,AND,OR,XOR} | BPF_FETCH and BPF_XCHG, and r0 for
BPF_CMPXCHG. emit_atomic_rmw() emits it that way, e.g.:

  [...]
  case BPF_XCHG:
          ctx->ex_insn_off = ctx->ninsns;
          emit(is64 ? rv_amoswap_d(rs, rs, rd, 1, 1) :
               rv_amoswap_w(rs, rs, rd, 1, 1), ctx);
  [...]

Thus, a fault over an unmapped arena page ex_handler_bpf() jumps over
the access but leaves rs untouched, and the program resumes with
whatever it held before the atomic instead of the 0 that every other
BPF_PROBE_* access delivers. Fill the exception table entry in from
bpf_atomic_load_reg(), which returns the BPF register an atomic reads
the memory operand into or -1 when it has none. A load-acquire ends up
with the same register it gets today, it just goes through the helper.
Unlike x86-64 and arm64, riscv64 does not report arena violations from
its exception handler, so there is no access direction to correct here,
only the missing register clear.

Fixes: fb7cefabae81 ("riscv, bpf: Add support arena atomics for RV64")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Pu Lehui <pulehui@huawei.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 6b9972b07c1b..2504df1fa111 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1992,10 +1992,19 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 			ret = emit_atomic_rmw(rd, rs, insn, ctx);
 
 		/* ret can be 1 (skip-zext); extable entry still needs to be added */
-		if (ret >= 0)
-			ret = add_exception_handler(insn,
-				bpf_atomic_is_load_acq(insn) ? rd : REG_DONT_CLEAR_MARKER,
-				ctx) ?: ret;
+		if (ret >= 0) {
+			/*
+			 * A load-acquire reads into dst_reg, and a read-modify-write
+			 * carrying BPF_FETCH reads the old value into src_reg, or into
+			 * r0 for a BPF_CMPXCHG. Clear that register on fault, the
+			 * remaining atomics have no destination register.
+			 */
+			int load_reg = bpf_atomic_load_reg(insn);
+
+			ret = add_exception_handler(insn, load_reg < 0 ?
+					REG_DONT_CLEAR_MARKER : regmap[load_reg],
+					ctx) ?: ret;
+		}
 
 		if (ret)
 			return ret;
-- 
2.43.0

[PATCH bpf-next v2 3/6] bpf, x86: Clear fetch destination on faulting arena atomic

[Daniel Borkmann]

populate_extable() encodes "there is no destination register to clear" as
DONT_CLEAR in the DST_REG field of the exception table metadata, and later
ex_handler_bpf() then reuses that very value to derive the direction it
reports the fault with is_write = (reg == DONT_CLEAR). The two coincide
for a plain load or store, but not for a RMW carrying BPF_FETCH. Such an
atomic writes memory, so it has to be reported as a WRITE, and it also reads
the old value into a register, src_reg for BPF_ADD | BPF_FETCH and BPF_XCHG,
r0 for BPF_CMPXCHG, so that register has to be cleared on fault. A single
DONT_CLEAR cannot say both, and the store branch picks it unconditionally:

  [...]
  } else {
          arena_reg = reg2pt_regs[dst_reg];
          fixup_reg = DONT_CLEAR;
  }
  [...]

The reported direction is therefore right, but on a fault over an unmapped
arena page the fetch destination keeps whatever it held before the atomic,
where every other BPF_PROBE_* access delivers 0. Give the metadata its own
ARENA_WRITE bit so that the reported direction no longer depends on whether
there is a register to clear, and fill DST_REG in from bpf_atomic_load_reg().
BPF_{AND,OR,XOR} | BPF_FETCH need no handling here, bpf_jit_supports_insn()
already rejects those in the arena.

Fixes: d503a04f8bc0 ("bpf: Add support for certain atomics in bpf_arena to x86 JIT")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 8dddb5d7af21..d920772af7d5 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1473,17 +1473,20 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
  *
  * Bit layout of `fixup` (32-bit):
  *
- * +-----------+--------+-----------+---------+----------+
- * | 31        | 30-24  |   23-16   |   15-8  |    7-0   |
- * |           |        |           |         |          |
- * | ARENA_ACC | Unused | ARENA_REG | DST_REG | INSN_LEN |
- * +-----------+--------+-----------+---------+----------+
+ * +-----------+-------------+--------+-----------+---------+----------+
+ * | 31        | 30          | 29-24  |   23-16   |   15-8  |    7-0   |
+ * |           |             |        |           |         |          |
+ * | 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)).
  * - 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 is a store.
+ *                      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
  *                       address for load/store when accessing the arena region.
+ * - ARENA_WRITE (1 bit): This bit is set when the faulting instruction wrote to the arena region.
+ *                        It is independent of DST_REG, since a read-modify-write both writes to
+ *                        memory and reads the old value into a register.
  * - ARENA_ACCESS (1 bit): This bit is set when the faulting instruction accessed the arena region.
  *
  * Bit layout of `data` (32-bit):
@@ -1502,6 +1505,7 @@ static int emit_atomic_ld_st_index(u8 **pprog, u32 atomic_op, u32 size,
 #define FIXUP_INSN_LEN_MASK	GENMASK(7, 0)
 #define FIXUP_REG_MASK		GENMASK(15, 8)
 #define FIXUP_ARENA_REG_MASK	GENMASK(23, 16)
+#define FIXUP_ARENA_WRITE	BIT(30)
 #define FIXUP_ARENA_ACCESS	BIT(31)
 #define DATA_ARENA_OFFSET_MASK	GENMASK(31, 16)
 
@@ -1510,7 +1514,7 @@ bool ex_handler_bpf(const struct exception_table_entry *x, struct pt_regs *regs)
 	u32 reg = FIELD_GET(FIXUP_REG_MASK, x->fixup);
 	u32 insn_len = FIELD_GET(FIXUP_INSN_LEN_MASK, x->fixup);
 	bool is_arena = !!(x->fixup & FIXUP_ARENA_ACCESS);
-	bool is_write = (reg == DONT_CLEAR);
+	bool is_write = !!(x->fixup & FIXUP_ARENA_WRITE);
 	unsigned long addr;
 	s16 off;
 	u32 arena_reg;
@@ -2348,6 +2352,7 @@ st:			insn_off = insn->off;
 				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)
@@ -2384,15 +2389,29 @@ st:			insn_off = insn->off;
 				    bpf_atomic_is_load_acq(insn)) {
 					arena_reg = reg2pt_regs[src_reg];
 					fixup_reg = reg2pt_regs[dst_reg];
+					is_write = false;
 				} else {
+					/*
+					 * A store has no destination register to clear,
+					 * except for a read-modify-write with BPF_FETCH,
+					 * which also reads the old value into src_reg, or
+					 * into r0 for a BPF_CMPXCHG. Either way the access
+					 * is still reported as a write.
+					 */
+					int load_reg = bpf_atomic_load_reg(insn);
+
 					arena_reg = reg2pt_regs[dst_reg];
-					fixup_reg = DONT_CLEAR;
+					fixup_reg = load_reg < 0 ? DONT_CLEAR :
+						    reg2pt_regs[load_reg];
+					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);
 			}
-- 
2.43.0

[PATCH bpf-next v2 4/6] bpf, arm64: Clear fetch destination on faulting arena atomic

[Daniel Borkmann]

Same problem as on x86-64: add_exception_handler() folds "there is no
destination register to clear" and "this is a store" into one DONT_CLEAR
value ...

  if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
          dst_reg = DONT_CLEAR;

... which ex_handler_bpf() then reads back as the access direction:

  bool is_write = (dst_reg == DONT_CLEAR);

A RMW carrying BPF_FETCH is both. emit_lse_atomic() reads the old value
into src_reg for BPF_{ADD,AND,OR,XOR} | BPF_FETCH and BPF_XCHG, and into
r0 for BPF_CMPXCHG, so a fault over an unmapped arena page is correctly
reported as a WRITE but leaves that register holding a stale value instead
of the 0 that every other BPF_PROBE_* access delivers. Same as on x86-64,
add a separate ARENA_WRITE bit for the direction.

FIXUP_REG is now filled in by the callers of add_exception_handler(), the
BPF_PROBE_ATOMIC one deriving it from bpf_atomic_load_reg(), so that the
helper only has to determine the direction. This is how the riscv64 JIT
already does it, and it stops the two store callers from handing in a
dst_reg that was only going to be overwritten with DONT_CLEAR anyway.

Fixes: e612b5c1d3ee ("bpf, arm64: Add support for lse atomics in bpf_arena")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Puranjay Mohan <puranjay@kernel.org>
---
 v1 -> v2:
   - move bpf_atomic_load_reg into build_insn (Eduard)

 arch/arm64/net/bpf_jit_comp.c | 44 ++++++++++++++++++++++++-----------
 1 file changed, 30 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index d14d297ebb96..74b4083791da 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1082,23 +1082,27 @@ static void build_epilogue(struct jit_ctx *ctx, bool was_classic)
  *
  * Bit layout of `fixup` (32-bit):
  *
- * +-----------+--------+-----------+-----------+----------+
- * |   31-27   | 26-22  |     21    |   20-16   |   15-0   |
- * |           |        |           |           |          |
- * | FIXUP_REG | Unused | ARENA_ACC | ARENA_REG |  OFFSET  |
- * +-----------+--------+-----------+-----------+----------+
+ * +-----------+--------+-------------+-----------+-----------+----------+
+ * |   31-27   | 26-23  |      22     |     21    |   20-16   |   15-0   |
+ * |           |        |             |           |           |          |
+ * | FIXUP_REG | Unused | ARENA_WRITE | ARENA_ACC | ARENA_REG |  OFFSET  |
+ * +-----------+--------+-------------+-----------+-----------+----------+
  *
  * - OFFSET (16 bits): Offset used to compute address for Load/Store instruction.
  * - ARENA_REG (5 bits): Register that is used to calculate the address for load/store when
  *                       accessing the arena region.
  * - ARENA_ACCESS (1 bit): This bit is set when the faulting instruction accessed the arena region.
+ * - ARENA_WRITE (1 bit): This bit is set when the faulting instruction wrote to the arena region.
+ *                        It is independent of FIXUP_REG, since a read-modify-write both writes to
+ *                        memory and reads the old value into a register.
  * - FIXUP_REG (5 bits): Destination register for the load instruction (cleared on fault) or set to
- *                       DONT_CLEAR if it is a store instruction.
+ *                       DONT_CLEAR if the instruction does not read into a register.
  */
 
 #define BPF_FIXUP_OFFSET_MASK      GENMASK(15, 0)
 #define BPF_FIXUP_ARENA_REG_MASK   GENMASK(20, 16)
 #define BPF_ARENA_ACCESS           BIT(21)
+#define BPF_ARENA_WRITE            BIT(22)
 #define BPF_FIXUP_REG_MASK	GENMASK(31, 27)
 #define DONT_CLEAR 5 /* Unused ARM64 register from BPF's POV */
 
@@ -1109,7 +1113,7 @@ bool ex_handler_bpf(const struct exception_table_entry *ex,
 	s16 off = FIELD_GET(BPF_FIXUP_OFFSET_MASK, ex->fixup);
 	int arena_reg = FIELD_GET(BPF_FIXUP_ARENA_REG_MASK, ex->fixup);
 	bool is_arena = !!(ex->fixup & BPF_ARENA_ACCESS);
-	bool is_write = (dst_reg == DONT_CLEAR);
+	bool is_write = !!(ex->fixup & BPF_ARENA_WRITE);
 	unsigned long addr;
 
 	if (is_arena) {
@@ -1132,7 +1136,7 @@ static int add_exception_handler(const struct bpf_insn *insn,
 {
 	off_t ins_offset;
 	s16 off = insn->off;
-	bool is_arena;
+	bool is_arena, is_write;
 	int arena_reg;
 	unsigned long pc;
 	struct exception_table_entry *ex;
@@ -1181,15 +1185,18 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	/*
 	 * A load-acquire is of BPF_STX class, but reads from src_reg into
 	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
-	 * here.
+	 * here. A read-modify-write carrying BPF_FETCH is reported as a write
+	 * even though it does have a register to clear, see the callers.
 	 */
-	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
-		dst_reg = DONT_CLEAR;
+	is_write = BPF_CLASS(insn->code) != BPF_LDX &&
+		   !bpf_atomic_is_load_acq(insn);
 
 	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
 
 	if (is_arena) {
 		ex->fixup |= BPF_ARENA_ACCESS;
+		if (is_write)
+			ex->fixup |= BPF_ARENA_WRITE;
 		/*
 		 * insn->src_reg/dst_reg holds the address in the arena region with upper 32-bits
 		 * being zero because of a preceding addr_space_cast(r<n>, 0x0, 0x1) instruction.
@@ -1889,7 +1896,7 @@ static int build_insn(const struct bpf_verifier_env *env, const struct bpf_insn
 			break;
 		}
 
-		ret = add_exception_handler(insn, ctx, dst);
+		ret = add_exception_handler(insn, ctx, DONT_CLEAR);
 		if (ret)
 			return ret;
 		break;
@@ -1956,7 +1963,7 @@ static int build_insn(const struct bpf_verifier_env *env, const struct bpf_insn
 			break;
 		}
 
-		ret = add_exception_handler(insn, ctx, dst);
+		ret = add_exception_handler(insn, ctx, DONT_CLEAR);
 		if (ret)
 			return ret;
 		break;
@@ -1979,7 +1986,16 @@ static int build_insn(const struct bpf_verifier_env *env, const struct bpf_insn
 			return ret;
 
 		if (BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
-			ret = add_exception_handler(insn, ctx, dst);
+			/*
+			 * A load-acquire reads into dst_reg, and a read-modify-write
+			 * carrying BPF_FETCH reads the old value into src_reg, or into
+			 * r0 for a BPF_CMPXCHG. Clear that register on fault, the
+			 * remaining atomics have no destination register.
+			 */
+			int load_reg = bpf_atomic_load_reg(insn);
+
+			ret = add_exception_handler(insn, ctx, load_reg < 0 ?
+						    DONT_CLEAR : bpf2a64[load_reg]);
 			if (ret)
 				return ret;
 		}
-- 
2.43.0

[PATCH bpf-next v2 5/6] bpf, s390: Clear fetch destination on faulting arena atomic

[Daniel Borkmann]

Same missing register clear as on riscv64. A RMW atomic on an arena pointer
is converted to BPF_PROBE_ATOMIC and gets an exception table entry, but
bpf_jit_probe_atomic_pre() only fills in the arena base and the probe
offset, leaving probe->reg at the -1 that bpf_jit_probe_init() set, which
bpf_jit_probe_post() writes into the entry and ex_handler_bpf() then reads
back as "there is nothing to clear".

That is right for a plain BPF_{ADD,AND,OR,XOR}, which only writes memory,
but an RMW carrying BPF_FETCH also reads the old value into a register:
src_reg for BPF_{ADD,AND,OR,XOR} | BPF_FETCH and BPF_XCHG, and r0 for
BPF_CMPXCHG. So on a fault over an unmapped arena page the program resumes
at the landing pad with whatever that register held before the atomic
instead of the 0 that every other BPF_PROBE_* access delivers.

Fill probe->reg in from bpf_atomic_load_reg(). Unlike x86-64 and arm64,
s390x does not report arena violations from its exception handler, so there
is no access direction to correct here, only the missing register clear.

Fixes: 2f9469484a3b ("s390/bpf: Support arena atomics")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Ilya Leoshkevich <iii@linux.ibm.com>
---
 arch/s390/net/bpf_jit_comp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index b60877478b45..c46872b071ce 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -774,6 +774,8 @@ static void bpf_jit_probe_atomic_pre(struct bpf_jit *jit,
 				     struct bpf_insn *insn,
 				     struct bpf_jit_probe *probe)
 {
+	int load_reg;
+
 	if (BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
 		return;
 
@@ -783,6 +785,14 @@ static void bpf_jit_probe_atomic_pre(struct bpf_jit *jit,
 	EMIT4(0xb9080000, REG_W1, insn->dst_reg);
 	probe->arena_reg = REG_W1;
 	probe->prg = jit->prg;
+	/*
+	 * A read-modify-write carrying BPF_FETCH reads the old value into
+	 * src_reg, or into r0 for a BPF_CMPXCHG. Clear that register on
+	 * fault, the remaining atomics only write memory.
+	 */
+	load_reg = bpf_atomic_load_reg(insn);
+	if (load_reg >= 0)
+		probe->reg = reg2hex[load_reg];
 }
 
 static int bpf_jit_probe_post(struct bpf_jit *jit, struct bpf_prog *fp,
@@ -1684,6 +1694,7 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, struct bpf_prog *fp,
 			if (load_probe.prg != -1) {
 				probe.prg = jit->prg;
 				probe.arena_reg = load_probe.arena_reg;
+				probe.reg = load_probe.reg;
 			}
 			loop_start = jit->prg;
 			/* 0: {csy|csg} %w0,%src,off(%arena) */
-- 
2.43.0

[PATCH bpf-next v2 6/6] selftests/bpf: Add arena fault tests for atomics with fetch

[Daniel Borkmann]

Add stream_arena_xchg_fault and stream_arena_cmpxchg_fault next to the
existing read, write and load-acquire fault tests, covering the two
places a read-modify-write can deposit the old value: src_reg for a
BPF_XCHG and r0 for a BPF_CMPXCHG. Both cover both halves of the JIT
bug that left the fetch destination alone when a RMW on an arena pointer
faulted:

  - the fault has to be reported as a WRITE, and at the address held by
    the destination register, which __stderr() and test_address() check
  - the register receiving the fetched value has to be cleared by the
    fault handler, which the programs check by poisoning it before the
    atomic and returning it, so __retval(0) fails if it is left untouched

The __stderr() annotation can only wildcard the faulting address since
the arena base is not known until runtime, hence the two test_address()
subtests on top, which pin it to the address held by dst_reg rather than
src_reg.

Note, the atomics are open coded since linux/filter.h cannot be included
alongside vmlinux.h.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t stream
  [...]
  #464/1   stream_arena_fault_address/read_fault:OK
  #464/2   stream_arena_fault_address/write_fault:OK
  #464/3   stream_arena_fault_address/load_acquire_fault:OK
  #464/4   stream_arena_fault_address/xchg_fault:OK
  #464/5   stream_arena_fault_address/cmpxchg_fault:OK
  #464     stream_arena_fault_address:OK
  [...]
  #466/5   stream_success/stream_arena_write_fault:OK
  #466/6   stream_success/stream_arena_read_fault:OK
  #466/7   stream_success/stream_arena_load_acquire_fault:OK
  #466/8   stream_success/stream_arena_xchg_fault:OK
  #466/9   stream_success/stream_arena_cmpxchg_fault:OK
  [...]
  Summary: 4/22 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
---
 v1 -> v2:
   - Updated commit msg wrt test_address structuring

 .../testing/selftests/bpf/prog_tests/stream.c |   4 +
 tools/testing/selftests/bpf/progs/stream.c    | 101 ++++++++++++++++++
 2 files changed, 105 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
index 15dd3ae2a84b..e4e9374309e2 100644
--- a/tools/testing/selftests/bpf/prog_tests/stream.c
+++ b/tools/testing/selftests/bpf/prog_tests/stream.c
@@ -105,6 +105,10 @@ void test_stream_arena_fault_address(void)
 		test_address(skel->progs.stream_arena_write_fault, &skel->bss->fault_addr);
 	if (test__start_subtest("load_acquire_fault"))
 		test_address(skel->progs.stream_arena_load_acquire_fault, &skel->bss->fault_addr);
+	if (test__start_subtest("xchg_fault"))
+		test_address(skel->progs.stream_arena_xchg_fault, &skel->bss->fault_addr);
+	if (test__start_subtest("cmpxchg_fault"))
+		test_address(skel->progs.stream_arena_cmpxchg_fault, &skel->bss->fault_addr);
 
 	stream__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
index cf5533e11f39..00a37933e411 100644
--- a/tools/testing/selftests/bpf/progs/stream.c
+++ b/tools/testing/selftests/bpf/progs/stream.c
@@ -229,6 +229,107 @@ int stream_arena_load_acquire_fault(void *ctx)
 	return val;
 }
 
+SEC("syscall")
+__arch_x86_64
+__arch_arm64
+__success __retval(0)
+__stderr("ERROR: Arena WRITE access at unmapped address 0x{{.*}}")
+__stderr("CPU: {{[0-9]+}} UID: 0 PID: {{[0-9]+}} Comm: {{.*}}")
+__stderr("Call trace:\n"
+"{{([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
+"|[ \t]+[^\n]+\n)*}}")
+int stream_arena_xchg_fault(void *ctx)
+{
+	static const struct bpf_insn xchg_insn = {
+		.code	 = 0xc3,	/* BPF_STX | BPF_ATOMIC | BPF_W */
+		.dst_reg = 1,		/* BPF_REG_1 */
+		.src_reg = 2,		/* BPF_REG_2 */
+		.off	 = 0x7fff,
+		.imm	 = 0xe1,	/* BPF_XCHG */
+	};
+	struct bpf_arena *ptr = (void *)&arena;
+	u64 user_vm_start, val;
+
+	/*
+	 * Prevent GCC bounds warning: casting &arena to struct bpf_arena *
+	 * triggers bounds checking since the map definition is smaller than
+	 * struct bpf_arena. barrier_var() makes the pointer opaque to GCC,
+	 * preventing the bounds analysis.
+	 */
+	barrier_var(ptr);
+	user_vm_start = ptr->user_vm_start;
+	fault_addr = user_vm_start + 0x7fff;
+	bpf_addr_space_cast(user_vm_start, 0, 1);
+	/*
+	 * A read-modify-write carrying BPF_FETCH writes to memory, so the fault
+	 * has to be reported as a WRITE from the dst_reg address, but it also
+	 * reads the old value into src_reg, so the exception handler has to
+	 * clear src_reg. Poison it up front, the returned value must be 0.
+	 */
+	asm volatile (
+		"r1 = %[user_vm_start];"
+		"r2 = 1;"
+		".8byte %[xchg_insn];" /* r2 = xchg((u32 *)(r1 + 0x7fff), r2) */
+		"%[val] = r2;"
+		: [val] "=r" (val)
+		: [user_vm_start] "r" (user_vm_start),
+		  __imm_insn(xchg_insn, xchg_insn)
+		: "r1", "r2"
+	);
+	return val;
+}
+
+SEC("syscall")
+__arch_x86_64
+__arch_arm64
+__success __retval(0)
+__stderr("ERROR: Arena WRITE access at unmapped address 0x{{.*}}")
+__stderr("CPU: {{[0-9]+}} UID: 0 PID: {{[0-9]+}} Comm: {{.*}}")
+__stderr("Call trace:\n"
+"{{([a-zA-Z_][a-zA-Z0-9_]*\\+0x[0-9a-fA-F]+/0x[0-9a-fA-F]+\n"
+"|[ \t]+[^\n]+\n)*}}")
+int stream_arena_cmpxchg_fault(void *ctx)
+{
+	static const struct bpf_insn cmpxchg_insn = {
+		.code	 = 0xc3,	/* BPF_STX | BPF_ATOMIC | BPF_W */
+		.dst_reg = 1,		/* BPF_REG_1 */
+		.src_reg = 2,		/* BPF_REG_2 */
+		.off	 = 0x7fff,
+		.imm	 = 0xf1,	/* BPF_CMPXCHG */
+	};
+	struct bpf_arena *ptr = (void *)&arena;
+	u64 user_vm_start, val;
+
+	/*
+	 * Prevent GCC bounds warning: casting &arena to struct bpf_arena *
+	 * triggers bounds checking since the map definition is smaller than
+	 * struct bpf_arena. barrier_var() makes the pointer opaque to GCC,
+	 * preventing the bounds analysis.
+	 */
+	barrier_var(ptr);
+	user_vm_start = ptr->user_vm_start;
+	fault_addr = user_vm_start + 0x7fff;
+	bpf_addr_space_cast(user_vm_start, 0, 1);
+	/*
+	 * Same as the exchange above, except that a BPF_CMPXCHG reads the old
+	 * value into r0 rather than into src_reg, so r0 is the register the
+	 * exception handler has to clear. It doubles as the compare value, but
+	 * the comparison never happens since the access faults first.
+	 */
+	asm volatile (
+		"r1 = %[user_vm_start];"
+		"r0 = 1;"
+		"r2 = 2;"
+		".8byte %[cmpxchg_insn];" /* r0 = cmpxchg((u32 *)(r1 + 0x7fff), r0, r2) */
+		"%[val] = r0;"
+		: [val] "=r" (val)
+		: [user_vm_start] "r" (user_vm_start),
+		  __imm_insn(cmpxchg_insn, cmpxchg_insn)
+		: "r0", "r1", "r2"
+	);
+	return val;
+}
+
 static __noinline void subprog(void)
 {
 	int __arena *addr = (int __arena *)0xdeadbeef;
-- 
2.43.0

Re: [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[bot+bpf-ci]

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

> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 4edba8182db1b..15d83684c6e94 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
>  	       insn->imm == BPF_LOAD_ACQ;
>  }
>
> +/*
> + * Given an instruction @insn, return the number of the BPF register that a
> + * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is
> + * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when
> + * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to
> + * be a BPF_ATOMIC here.
> + */
> +static inline int bpf_atomic_load_reg(const struct bpf_insn *insn)
> +{
> +	if (BPF_CLASS(insn->code) != BPF_STX ||
> +	    (BPF_MODE(insn->code) != BPF_ATOMIC &&
> +	     BPF_MODE(insn->code) != BPF_PROBE_ATOMIC))
> +		return -1;
> +
> +	switch (insn->imm) {
> +	case BPF_LOAD_ACQ:
> +		return insn->dst_reg;
> +	case BPF_CMPXCHG:
> +		return BPF_REG_0;
> +	default:
> +		return (insn->imm & BPF_FETCH) ? insn->src_reg : -1;
> +	}
> +}
> +
>  /* Memory store, *(uint *) (dst_reg + off16) = imm32 */
>
>  #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 661e2d13a6042..c4bd70befbb5c 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -49,16 +49,7 @@ static int insn_def_regno(const struct bpf_insn *insn)
>  	case BPF_ST:
>  		return -1;
>  	case BPF_STX:
> -		if (BPF_MODE(insn->code) == BPF_ATOMIC ||
> -		    BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
> -			if (insn->imm == BPF_CMPXCHG)
> -				return BPF_REG_0;
> -			else if (insn->imm == BPF_LOAD_ACQ)
> -				return insn->dst_reg;
> -			else if (insn->imm & BPF_FETCH)
> -				return insn->src_reg;
> -		}
> -		return -1;
> +		return bpf_atomic_load_reg(insn);
>  	default:
>  		return insn->dst_reg;
>  	}
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index add3affc57035..73a2e8bb1782a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6485,21 +6485,16 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>
> -	if (insn->imm & BPF_FETCH) {
> -		if (insn->imm == BPF_CMPXCHG)
> -			load_reg = BPF_REG_0;
> -		else
> -			load_reg = insn->src_reg;
> -
> +	/*
> +	 * A negative load_reg means that this instruction accesses a memory
> +	 * location but doesn't actually load it into a register.
> +	 */
> +	load_reg = bpf_atomic_load_reg(insn);
> +	if (load_reg >= 0) {
>  		/* check and record load of old value */
>  		err = check_reg_arg(env, load_reg, DST_OP);
>  		if (err)
>  			return err;
> -	} else {
> -		/* This instruction accesses a memory location but doesn't
> -		 * actually load it into a register.
> -		 */
> -		load_reg = -1;
>  	}
>
>  	dst_reg = cur_regs(env) + insn->dst_reg;

This isn't a bug, but since the point of bpf_atomic_load_reg() is to keep
this derivation in one place, would it be worth converting the remaining
instances in const_fold.c and liveness.c as well?

Both files have the same three-way BPF_CMPXCHG/BPF_LOAD_ACQ/BPF_FETCH
mapping behind a BPF_STX + BPF_MODE == BPF_ATOMIC guard. In const_fold.c
around line 204-209 it marks the register as unknown, and in liveness.c
around line 1212-1217 it marks the register as none. Each could read as
'int r = bpf_atomic_load_reg(insn); if (r >= 0) <clear r>;' instead of
repeating the switch.


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

Re: [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic

[Pu Lehui]

Hi Daniel,

On 2026/8/11 6:18, Daniel Borkmann wrote:
> A RMW atomic on an arena pointer is converted to BPF_PROBE_ATOMIC and
> gets an exception table entry, but that entry records no destination
> register to clear unless the instruction is a load-acquire today. That
> is right for a plain BPF_{ADD,AND,OR,XOR}, which only writes memory,
> but an RMW carrying BPF_FETCH also reads the old value into a register:
> src_reg for BPF_{ADD,AND,OR,XOR} | BPF_FETCH and BPF_XCHG, and r0 for
> BPF_CMPXCHG. emit_atomic_rmw() emits it that way, e.g.:
> 
>    [...]
>    case BPF_XCHG:
>            ctx->ex_insn_off = ctx->ninsns;
>            emit(is64 ? rv_amoswap_d(rs, rs, rd, 1, 1) :
>                 rv_amoswap_w(rs, rs, rd, 1, 1), ctx);
>    [...]
> 
> Thus, a fault over an unmapped arena page ex_handler_bpf() jumps over
> the access but leaves rs untouched, and the program resumes with
> whatever it held before the atomic instead of the 0 that every other
> BPF_PROBE_* access delivers. Fill the exception table entry in from
> bpf_atomic_load_reg(), which returns the BPF register an atomic reads
> the memory operand into or -1 when it has none. A load-acquire ends up
> with the same register it gets today, it just goes through the helper.
> Unlike x86-64 and arm64, riscv64 does not report arena violations from
> its exception handler, so there is no access direction to correct here,
> only the missing register clear.
> 
> Fixes: fb7cefabae81 ("riscv, bpf: Add support arena atomics for RV64")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Pu Lehui <pulehui@huawei.com>
> ---
>   arch/riscv/net/bpf_jit_comp64.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 6b9972b07c1b..2504df1fa111 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1992,10 +1992,19 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   			ret = emit_atomic_rmw(rd, rs, insn, ctx);
>   
>   		/* ret can be 1 (skip-zext); extable entry still needs to be added */
> -		if (ret >= 0)
> -			ret = add_exception_handler(insn,
> -				bpf_atomic_is_load_acq(insn) ? rd : REG_DONT_CLEAR_MARKER,
> -				ctx) ?: ret;
> +		if (ret >= 0) {
> +			/*
> +			 * A load-acquire reads into dst_reg, and a read-modify-write
> +			 * carrying BPF_FETCH reads the old value into src_reg, or into
> +			 * r0 for a BPF_CMPXCHG. Clear that register on fault, the
> +			 * remaining atomics have no destination register.
> +			 */
> +			int load_reg = bpf_atomic_load_reg(insn);
> +
> +			ret = add_exception_handler(insn, load_reg < 0 ?
> +					REG_DONT_CLEAR_MARKER : regmap[load_reg],

Felt a bit odd using regmap directly here at first, but since 
insn->dst_reg and insn->src_reg are already marked via bpf_to_rv_reg at 
the start, it looks fine.

Reviewed-by: Pu Lehui <pulehui@huawei.com>

Thanks.

> +					ctx) ?: ret;
> +		}
>   
>   		if (ret)
>   			return ret;

Re: [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[Daniel Borkmann]

On 8/11/26 1:38 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index 4edba8182db1b..15d83684c6e94 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
>>   	       insn->imm == BPF_LOAD_ACQ;
>>   }
>>
>> +/*
>> + * Given an instruction @insn, return the number of the BPF register that a
>> + * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is
>> + * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when
>> + * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to
>> + * be a BPF_ATOMIC here.
>> + */
>> +static inline int bpf_atomic_load_reg(const struct bpf_insn *insn)
>> +{
>> +	if (BPF_CLASS(insn->code) != BPF_STX ||
>> +	    (BPF_MODE(insn->code) != BPF_ATOMIC &&
>> +	     BPF_MODE(insn->code) != BPF_PROBE_ATOMIC))
>> +		return -1;
>> +
>> +	switch (insn->imm) {
>> +	case BPF_LOAD_ACQ:
>> +		return insn->dst_reg;
>> +	case BPF_CMPXCHG:
>> +		return BPF_REG_0;
>> +	default:
>> +		return (insn->imm & BPF_FETCH) ? insn->src_reg : -1;
>> +	}
>> +}
>> +
>>   /* Memory store, *(uint *) (dst_reg + off16) = imm32 */
>>
>>   #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index 661e2d13a6042..c4bd70befbb5c 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
>> @@ -49,16 +49,7 @@ static int insn_def_regno(const struct bpf_insn *insn)
>>   	case BPF_ST:
>>   		return -1;
>>   	case BPF_STX:
>> -		if (BPF_MODE(insn->code) == BPF_ATOMIC ||
>> -		    BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
>> -			if (insn->imm == BPF_CMPXCHG)
>> -				return BPF_REG_0;
>> -			else if (insn->imm == BPF_LOAD_ACQ)
>> -				return insn->dst_reg;
>> -			else if (insn->imm & BPF_FETCH)
>> -				return insn->src_reg;
>> -		}
>> -		return -1;
>> +		return bpf_atomic_load_reg(insn);
>>   	default:
>>   		return insn->dst_reg;
>>   	}
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index add3affc57035..73a2e8bb1782a 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -6485,21 +6485,16 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
>>   		return -EACCES;
>>   	}
>>
>> -	if (insn->imm & BPF_FETCH) {
>> -		if (insn->imm == BPF_CMPXCHG)
>> -			load_reg = BPF_REG_0;
>> -		else
>> -			load_reg = insn->src_reg;
>> -
>> +	/*
>> +	 * A negative load_reg means that this instruction accesses a memory
>> +	 * location but doesn't actually load it into a register.
>> +	 */
>> +	load_reg = bpf_atomic_load_reg(insn);
>> +	if (load_reg >= 0) {
>>   		/* check and record load of old value */
>>   		err = check_reg_arg(env, load_reg, DST_OP);
>>   		if (err)
>>   			return err;
>> -	} else {
>> -		/* This instruction accesses a memory location but doesn't
>> -		 * actually load it into a register.
>> -		 */
>> -		load_reg = -1;
>>   	}
>>
>>   	dst_reg = cur_regs(env) + insn->dst_reg;
> 
> This isn't a bug, but since the point of bpf_atomic_load_reg() is to keep
> this derivation in one place, would it be worth converting the remaining
> instances in const_fold.c and liveness.c as well?
> 
> Both files have the same three-way BPF_CMPXCHG/BPF_LOAD_ACQ/BPF_FETCH
> mapping behind a BPF_STX + BPF_MODE == BPF_ATOMIC guard. In const_fold.c
> around line 204-209 it marks the register as unknown, and in liveness.c
> around line 1212-1217 it marks the register as none. Each could read as
> 'int r = bpf_atomic_load_reg(insn); if (r >= 0) <clear r>;' instead of
> repeating the switch.
Ack, I have it locally in a v3 of the series.

Re: [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[Jakub Sitnicki]

On Tue, Aug 11, 2026 at 12:18 AM +02, Daniel Borkmann wrote:
> check_atomic_rmw() open codes the mapping from a BPF_ATOMIC to the register
> it reads the old value into, the BPF_STX case of insn_def_regno() open codes
> the very same mapping a second time, and BPF JITs need it as well to know
> which register a faulting BPF_PROBE_ATOMIC has to clear. Having the
> derivations sit in different files is how the JITs came to disagree with
> the verifier in the first place. Add a small helper so that all of them can
> share it. No functional change. The BPF_LOAD_ACQ case is there for the JITs,
> which do walk all instruction classes.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
>  v1 -> v2:
>    - also convert insn_def_regno (Eduard, sashiko)
>
>  include/linux/filter.h | 24 ++++++++++++++++++++++++
>  kernel/bpf/fixups.c    | 11 +----------
>  kernel/bpf/verifier.c  | 17 ++++++-----------
>  3 files changed, 31 insertions(+), 21 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 4edba8182db1..15d83684c6e9 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -414,6 +414,30 @@ static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
>  	       insn->imm == BPF_LOAD_ACQ;
>  }
>  
> +/*
> + * Given an instruction @insn, return the number of the BPF register that a
> + * BPF_ATOMIC reads the value at its memory operand into, or -1 if there is
> + * no such register. That is the register a BPF_PROBE_ATOMIC has to clear when
> + * the access faults. Like bpf_atomic_is_load_acq(), @insn is not assumed to
> + * be a BPF_ATOMIC here.
> + */
> +static inline int bpf_atomic_load_reg(const struct bpf_insn *insn)
> +{
> +	if (BPF_CLASS(insn->code) != BPF_STX ||
> +	    (BPF_MODE(insn->code) != BPF_ATOMIC &&
> +	     BPF_MODE(insn->code) != BPF_PROBE_ATOMIC))
> +		return -1;
> +
> +	switch (insn->imm) {
> +	case BPF_LOAD_ACQ:
> +		return insn->dst_reg;
> +	case BPF_CMPXCHG:
> +		return BPF_REG_0;
> +	default:
> +		return (insn->imm & BPF_FETCH) ? insn->src_reg : -1;
> +	}
> +}
> +
>  /* Memory store, *(uint *) (dst_reg + off16) = imm32 */
>  
>  #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 661e2d13a604..c4bd70befbb5 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -49,16 +49,7 @@ static int insn_def_regno(const struct bpf_insn *insn)
>  	case BPF_ST:
>  		return -1;
>  	case BPF_STX:
> -		if (BPF_MODE(insn->code) == BPF_ATOMIC ||
> -		    BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) {
> -			if (insn->imm == BPF_CMPXCHG)
> -				return BPF_REG_0;
> -			else if (insn->imm == BPF_LOAD_ACQ)
> -				return insn->dst_reg;
> -			else if (insn->imm & BPF_FETCH)
> -				return insn->src_reg;
> -		}
> -		return -1;
> +		return bpf_atomic_load_reg(insn);
>  	default:
>  		return insn->dst_reg;
>  	}
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index add3affc5703..73a2e8bb1782 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -6485,21 +6485,16 @@ static int check_atomic_rmw(struct bpf_verifier_env *env,
>  		return -EACCES;
>  	}
>  
> -	if (insn->imm & BPF_FETCH) {
> -		if (insn->imm == BPF_CMPXCHG)
> -			load_reg = BPF_REG_0;
> -		else
> -			load_reg = insn->src_reg;
> -
> +	/*
> +	 * A negative load_reg means that this instruction accesses a memory
> +	 * location but doesn't actually load it into a register.
> +	 */

Nit: This sounds like it's part of bpf_atomic_load_reg doc. Describes
the return value. Consider moving.

> +	load_reg = bpf_atomic_load_reg(insn);
> +	if (load_reg >= 0) {
>  		/* check and record load of old value */
>  		err = check_reg_arg(env, load_reg, DST_OP);
>  		if (err)
>  			return err;
> -	} else {
> -		/* This instruction accesses a memory location but doesn't
> -		 * actually load it into a register.
> -		 */
> -		load_reg = -1;
>  	}
>  
>  	dst_reg = cur_regs(env) + insn->dst_reg;

Re: [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place

[Eduard Zingerman]

On Tue, 2026-08-11 at 00:18 +0200, Daniel Borkmann wrote:
> check_atomic_rmw() open codes the mapping from a BPF_ATOMIC to the register
> it reads the old value into, the BPF_STX case of insn_def_regno() open codes
> the very same mapping a second time, and BPF JITs need it as well to know
> which register a faulting BPF_PROBE_ATOMIC has to clear. Having the
> derivations sit in different files is how the JITs came to disagree with
> the verifier in the first place. Add a small helper so that all of them can
> share it. No functional change. The BPF_LOAD_ACQ case is there for the JITs,
> which do walk all instruction classes.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

...