github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-09-22 04:34:03 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

bpf, x86: Clear fetch destination on faulting arena atomic

Browse Source 
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: d503a04f8b ("bpf: Add support for certain atomics in bpf_arena to x86 JIT")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://patch.msgid.link/20260811131600.506721-3-daniel@iogearbox.net
Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
This commit is contained in:
Daniel Borkmann 2026-08-11 15:15:57 +02:00 committed by Eduard Zingerman
parent 1519f488e8
commit cf92a10860
1 changed files with 27 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
arch/x86/net/bpf_jit_comp.c
View File

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