* [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
@ 2026-08-10 22:18 ` Daniel Borkmann
2026-08-11 2:30 ` Pu Lehui
2026-08-10 22:18 ` [PATCH bpf-next v2 3/6] bpf, x86: " Daniel Borkmann
` (4 subsequent siblings)
5 siblings, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2026-08-10 22:18 UTC (permalink / raw)
To: memxor; +Cc: eddyz87, puranjay, bpf, Pu Lehui
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic
2026-08-10 22:18 ` [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic Daniel Borkmann
@ 2026-08-11 2:30 ` Pu Lehui
0 siblings, 0 replies; 8+ messages in thread
From: Pu Lehui @ 2026-08-11 2:30 UTC (permalink / raw)
To: Daniel Borkmann, memxor; +Cc: eddyz87, puranjay, bpf
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;
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH bpf-next v2 3/6] bpf, x86: Clear fetch destination on faulting arena atomic
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic Daniel Borkmann
@ 2026-08-10 22:18 ` Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 4/6] bpf, arm64: " Daniel Borkmann
` (3 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2026-08-10 22:18 UTC (permalink / raw)
To: memxor; +Cc: eddyz87, puranjay, bpf
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 4/6] bpf, arm64: Clear fetch destination on faulting arena atomic
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 2/6] bpf, riscv: Clear fetch destination on faulting arena atomic Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 3/6] bpf, x86: " Daniel Borkmann
@ 2026-08-10 22:18 ` Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 5/6] bpf, s390: " Daniel Borkmann
` (2 subsequent siblings)
5 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2026-08-10 22:18 UTC (permalink / raw)
To: memxor; +Cc: eddyz87, puranjay, bpf
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 5/6] bpf, s390: Clear fetch destination on faulting arena atomic
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
` (2 preceding siblings ...)
2026-08-10 22:18 ` [PATCH bpf-next v2 4/6] bpf, arm64: " Daniel Borkmann
@ 2026-08-10 22:18 ` Daniel Borkmann
2026-08-10 22:18 ` [PATCH bpf-next v2 6/6] selftests/bpf: Add arena fault tests for atomics with fetch Daniel Borkmann
2026-08-10 23:38 ` [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place bot+bpf-ci
5 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2026-08-10 22:18 UTC (permalink / raw)
To: memxor; +Cc: eddyz87, puranjay, bpf, Ilya Leoshkevich
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH bpf-next v2 6/6] selftests/bpf: Add arena fault tests for atomics with fetch
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
` (3 preceding siblings ...)
2026-08-10 22:18 ` [PATCH bpf-next v2 5/6] bpf, s390: " Daniel Borkmann
@ 2026-08-10 22:18 ` Daniel Borkmann
2026-08-10 23:38 ` [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place bot+bpf-ci
5 siblings, 0 replies; 8+ messages in thread
From: Daniel Borkmann @ 2026-08-10 22:18 UTC (permalink / raw)
To: memxor; +Cc: eddyz87, puranjay, bpf
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place
2026-08-10 22:18 [PATCH bpf-next v2 1/6] bpf: Derive the atomic load register in one place Daniel Borkmann
` (4 preceding siblings ...)
2026-08-10 22:18 ` [PATCH bpf-next v2 6/6] selftests/bpf: Add arena fault tests for atomics with fetch Daniel Borkmann
@ 2026-08-10 23:38 ` bot+bpf-ci
5 siblings, 0 replies; 8+ messages in thread
From: bot+bpf-ci @ 2026-08-10 23:38 UTC (permalink / raw)
To: daniel, memxor
Cc: eddyz87, puranjay, bpf, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- 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
^ permalink raw reply [flat|nested] 8+ messages in thread