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

[PATCH bpf-next 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, and BPF JITs need the very same mapping to know
which register a faulting BPF_PROBE_ATOMIC has to clear. Having the two
derivations sit in different files is how the JITs came to disagree with
the verifier in the first place. Add a small helper so it can be reused.
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>
---
 include/linux/filter.h | 24 ++++++++++++++++++++++++
 kernel/bpf/verifier.c  | 17 ++++++-----------
 2 files changed, 30 insertions(+), 11 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/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 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 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>
---
 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 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 and fill FIXUP_REG in
from bpf_atomic_load_reg().

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>
---
 arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index d14d297ebb96..796ff9193cfb 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 = false;
 	int arena_reg;
 	unsigned long pc;
 	struct exception_table_entry *ex;
@@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
 	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
 	 * here.
 	 */
-	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
-		dst_reg = DONT_CLEAR;
+	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
+		/*
+		 * 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);
+
+		dst_reg = load_reg < 0 ? DONT_CLEAR : bpf2a64[load_reg];
+		is_write = true;
+	}
 
 	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.
-- 
2.43.0

[PATCH bpf-next 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 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

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>
---
 .../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 3/6] bpf, x86: Clear fetch destination on faulting arena atomic

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The fix for clearing the fetch destination register on a faulting arena read-modify-write atomic is incomplete and misses arm64 and s390 architectures.
--

--- Patch [3]: [PATCH bpf-next 3/6] bpf, x86: Clear fetch destination on faulting arena atomic ---
commit d051fde5d669db0d464e3f70d0cdcf647b528ab3
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf, x86: Clear fetch destination on faulting arena atomic

This commit fixes an issue on x86 where faulting arena read-modify-write
atomic instructions with a fetch operation failed to clear the destination
register. It introduces a new ARENA_WRITE bit in the exception table
metadata to correctly report the fault direction independently of the
register clearing logic.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 8dddb5d7af21b..d920772af7d5f 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c

[Severity: High]
This is a pre-existing issue, but does this same bug also need to be fixed
in the arm64 and s390 JIT implementations?

On arm64, add_exception_handler() hardcodes DONT_CLEAR for all instructions
except BPF_LDX and BPF_LOAD_ACQ, which seems to leave the fetch register
uncleared for faulting BPF_PROBE_ATOMIC instructions:

arch/arm64/net/bpf_jit_comp.c:add_exception_handler() {
	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
		dst_reg = DONT_CLEAR;

	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
}

On s390, bpf_jit_probe_atomic_pre() never sets probe->reg, leaving it
initialized to -1. This seems to cause ex_handler_bpf() to skip clearing any
register:

arch/s390/net/bpf_jit_comp.c:bpf_jit_probe_atomic_pre() {
	if (BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
		return;

	/* lgrl %r1,kern_arena */
	EMIT6_PCREL_RILB(0xc4080000, REG_W1, jit->kern_arena);
	/* agr %r1,%dst */
	EMIT4(0xb9080000, REG_W1, insn->dst_reg);
	probe->arena_reg = REG_W1;
	probe->prg = jit->prg;
}

Could this result in leaking stale or uninitialized memory to the BPF
program when an arena atomic read-modify-write instruction faults on an
unmapped page on these architectures?

> @@ -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;
>  				}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810134346.466004-1-daniel@iogearbox.net?part=3

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

[Daniel Borkmann]

On 8/10/26 4:07 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] The fix for clearing the fetch destination register on a faulting arena read-modify-write atomic is incomplete and misses arm64 and s390 architectures.
(done in the other patches of this series)

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2278 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 */

This isn't a bug, but could insn_def_regno()'s BPF_STX arm (and the
similar switches in const_fold.c and liveness.c) call
bpf_atomic_load_reg(), so the mapping really does live in one place?

The helper is bit-for-bit equivalent to the BPF_STX arm of
insn_def_regno() in kernel/bpf/fixups.c:

	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;

The same three-way mapping is also open coded in
kernel/bpf/const_fold.c and kernel/bpf/liveness.c.

Since the changelog's stated goal is that the derivation should live in
one place, it is a little surprising that the in-tree duplicates are
left alone.

[ ... ]


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

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

[Eduard Zingerman]

On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:

...

> --- 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;
> +	}
> +}
> +

Hi Daniel,

Note that there is already a very similar fixups.c:insn_def_regno().
Should we try to avoid duplication?

...

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

[Eduard Zingerman]

On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:
> 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>

...

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

[Daniel Borkmann]

On 8/10/26 7:10 PM, Eduard Zingerman wrote:
> On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:
> 
> ...
> 
>> --- 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;
>> +	}
>> +}
>> +
> 
> Hi Daniel,
> 
> Note that there is already a very similar fixups.c:insn_def_regno().
> Should we try to avoid duplication?
> 
> ...

Yeap, I'll look into it to consolidate.

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

[Eduard Zingerman]

On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:
> 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 and fill FIXUP_REG in
> from bpf_atomic_load_reg().
> 
> 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>
> ---

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

>  arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index d14d297ebb96..796ff9193cfb 100644
> --- a/arch/arm64/net/bpf_jit_comp.c
> +++ b/arch/arm64/net/bpf_jit_comp.c

...

> @@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
>  	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
>  	 * here.
>  	 */
> -	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
> -		dst_reg = DONT_CLEAR;
> +	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
> +		/*
> +		 * 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);

Nit: I think it would be more in line with the current arm64 jit organization
     if bpf_atomic_load_reg() call is moved to the add_exception_handler()
     callsite in the build_insn(), where it handles BPF_PROBE_ATOMIC.

> +
> +		dst_reg = load_reg < 0 ? DONT_CLEAR : bpf2a64[load_reg];
> +		is_write = true;
> +	}
>  
>  	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.

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

[Puranjay Mohan]

Daniel Borkmann <daniel@iogearbox.net> writes:

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

Reviewed-by: Puranjay Mohan <puranjay@kernel.org>

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

[Puranjay Mohan]

Eduard Zingerman <eddyz87@gmail.com> writes:

> On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:
>> 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 and fill FIXUP_REG in
>> from bpf_atomic_load_reg().
>> 
>> 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>
>> ---
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
>>  arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
>>  1 file changed, 26 insertions(+), 10 deletions(-)
>> 
>> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
>> index d14d297ebb96..796ff9193cfb 100644
>> --- a/arch/arm64/net/bpf_jit_comp.c
>> +++ b/arch/arm64/net/bpf_jit_comp.c
>
> ...
>
>> @@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
>>  	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
>>  	 * here.
>>  	 */
>> -	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
>> -		dst_reg = DONT_CLEAR;
>> +	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
>> +		/*
>> +		 * 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);
>
> Nit: I think it would be more in line with the current arm64 jit organization
>      if bpf_atomic_load_reg() call is moved to the add_exception_handler()
>      callsite in the build_insn(), where it handles BPF_PROBE_ATOMIC.
>

Wouldn't that cause more code churn?

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

[Puranjay Mohan]

Daniel Borkmann <daniel@iogearbox.net> writes:

> 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 and fill FIXUP_REG in
> from bpf_atomic_load_reg().
>
> 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>
> ---
>  arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
>  1 file changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> index d14d297ebb96..796ff9193cfb 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 = false;
>  	int arena_reg;
>  	unsigned long pc;
>  	struct exception_table_entry *ex;
> @@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
>  	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
>  	 * here.
>  	 */
> -	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
> -		dst_reg = DONT_CLEAR;
> +	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
> +		/*
> +		 * 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);
> +
> +		dst_reg = load_reg < 0 ? DONT_CLEAR : bpf2a64[load_reg];
> +		is_write = true;
> +	}
>  
>  	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.
> -- 
> 2.43.0


Reviewed-by: Puranjay Mohan <puranjay@kernel.org>

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

[Eduard Zingerman]

On Mon, 2026-08-10 at 19:30 +0100, Puranjay Mohan wrote:

...

> > >  arch/arm64/net/bpf_jit_comp.c | 36 +++++++++++++++++++++++++----------
> > >  1 file changed, 26 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
> > > index d14d297ebb96..796ff9193cfb 100644
> > > --- a/arch/arm64/net/bpf_jit_comp.c
> > > +++ b/arch/arm64/net/bpf_jit_comp.c
> > 
> > ...
> > 
> > > @@ -1183,13 +1187,25 @@ static int add_exception_handler(const struct bpf_insn *insn,
> > >  	 * dst_reg like a BPF_LDX does, hence it must not be treated as a store
> > >  	 * here.
> > >  	 */
> > > -	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
> > > -		dst_reg = DONT_CLEAR;
> > > +	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn)) {
> > > +		/*
> > > +		 * 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);
> > 
> > Nit: I think it would be more in line with the current arm64 jit organization
> >      if bpf_atomic_load_reg() call is moved to the add_exception_handler()
> >      callsite in the build_insn(), where it handles BPF_PROBE_ATOMIC.
> > 
> 
> Wouldn't that cause more code churn?

Having 'dst' both passed as a parameter and passed from the callsite
is somewhat inconsistent.

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

[Eduard Zingerman]

On Mon, 2026-08-10 at 15:43 +0200, Daniel Borkmann wrote:
> 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
> 
> 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>

...

> 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);

Tbh, this seem to be an overkill. The point of these stream tests is
to check the stream output format details etc. Here the __stderr
annotation in the test already captures that the fault had happened
and also checks retval.

...