bpf, arm64: Clear fetch destination on faulting arena atomic

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: e612b5c1d3 ("bpf, arm64: Add support for lse atomics in bpf_arena")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
Link: https://patch.msgid.link/20260811131600.506721-4-daniel@iogearbox.net
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
This commit is contained in:
Daniel Borkmann 2026-08-11 15:15:58 +02:00 committed by Eduard Zingerman
parent cf92a10860
commit ea3f20cb59

View File

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