Re: [PATCH bpf-next v5 1/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena

Re: [PATCH bpf-next v5 1/3] bpf, x86: Support fetching AND/OR/XOR atomics in arena

[Tejun Heo]

On Fri, Oct 16, 2026 at 10:13:12AM -0700, Puranjay Mohan wrote:
> x86-64 has no single instruction for a fetching AND/OR/XOR, so the JIT
> lowers them to a CMPXCHG loop. That loop could not be used against arena
> memory: it contains two memory accesses, the load of the old value and
> the CMPXCHG itself, either of which can fault when the arena page goes
> away, while the verifier reserves only one exception table entry per
> instruction. bpf_jit_supports_insn() therefore rejected the three opcodes
> and such programs failed to load with
>
>   BPF_ATOMIC stores into R1 arena is not allowed
>
> Emit the same loop with R12-indexed addressing and give each of the two
> accesses its own exception table entry. The verifier still reserves one
> per instruction, so the JIT accounts for the extra entry itself by
> rescanning the instruction stream in bpf_int_jit_compile() before the
> extable is sized, as s390 does in bpf_jit_alloc() for its BPF_XCHG
> lowering. The rescan sits after the extra-pass early exit, so a program
> is counted exactly once. Drop the bpf_jit_supports_insn() rejection.
>
> A loop without the load would need only one entry, since a CMPXCHG that
> loses the comparison reloads RAX, but its first CMPXCHG would then almost
> always lose. Every operation would pay for a second locked CMPXCHG, which
> makes the fetching insn more expensive than the load and CMPXCHG loop
> programs write by hand today. The second entry is paid for once, in the
> JIT.
>
> Both entries resume past the whole loop rather than past the faulting
> instruction alone, with the fetch destination cleared, so a fault cannot
> re-enter the loop, and both are reported as writes since the BPF insn is
> a read-modify-write. This repurposes the INSN_LEN field of the fixup as a
> resume distance rather than an instruction length; a sequence long enough
> to overflow its 8 bits would first have to exceed BPF_MAX_INSN_SIZE, which
> do_jit() rejects before the image is used.
>
> The loop needs RAX for CMPXCHG and saves R0 in BPF_REG_AX, which it also
> substitutes for R0 when either operand is R0. ex_handler_bpf() may then
> have to name r10 as the register holding the arena address or the one to
> clear on fault; reg2pt_regs[] already has that entry.
>
> An access through a number, made by a program loaded with
> BPF_F_ARENA_SCALAR, keeps its address in BPF_REG_AX itself (see
> arena_scalar_access()), which is where the loop would save R0. Save R0 on
> the stack in that case, as the div and mod lowering does. An R0 operand
> stays in that stack slot: the new value is built from it, RAX ends up
> holding the old value that R0 is to return, and the slot is dropped
> instead of popped. Both entries resume before that pop or drop, so a
> fault leaves the stack balanced.
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

Re: [PATCH bpf-next v5 2/3] selftests/bpf: Test fetching AND/OR/XOR atomics in arena

[Tejun Heo]

On Fri, Oct 16, 2026 at 10:13:13AM -0700, Puranjay Mohan wrote:
> The arena and/or/xor tests never exercised the fetching form of the
> instruction. Commit 2897b1e2a2f4 ("selftests/bpf: Fix arena_atomics
> failure due to llvm change") deliberately switched them to
> __c11_atomic_fetch_*() with memory_order_relaxed, which clang lowers to a
> non-fetching locked instruction when the result is unused, because
> x86-64 could not JIT the fetching one against an arena.
>
> It can now, so check the returned old value with __sync_fetch_and_*().
> Keep a __c11_atomic_fetch_*() of each width per operation with the result
> discarded: that spelling is what selects the non-fetching insn, which
> would otherwise lose its only coverage, on every architecture rather than
> just x86. Compiling the prog with clang 22 before and after confirms the
> split: 6 non-fetching and no fetching arena and/or/xor before, 6 and 22
> after.
>
> The new globals go in their operation's block, and the or block keeps its
> u32s first, so .addr_space.1 stays free of the padding holes that clang
> before 18 cannot fill when it is built without address space casts.
>
> Add a fetch_r0 test for the two register assignments the x86 JIT has to
> special-case, where the operand and where the arena pointer is R0 and
> BPF_REG_AX is substituted for it. Clang picks its own registers and will
> not reliably produce either, so spell the instructions out. The values
> have bits set above 32 so that a truncating move on that path shows up.
> Add both forms to the uaf test as well, since the substitution only
> reaches the exception table when such an access faults, and check that
> the fault cleared the fetched register and left the address alone. They
> fault on the page after the freed one: the freed page is the first of the
> arena, whose 32-bit address is 0, which would make the address check
> empty. The address is read back afterwards, which gives 0 if the JIT's
> exception table entry recovered the fault, but not if the arena had
> mapped its zeroed scratch page and let the access succeed. The powerpc64
> JIT does not clear the register of a faulting atomic yet, so those checks
> are skipped there.
>
> Both spell addr_space_cast textually, which needs LLVM 19, so uaf gains
> the __BPF_FEATURE_ADDR_SPACE_CAST guard that every other textual use in
> the file already has. It was plain C before and built on older clang.
>
> Drop x86 from the exclusion list of the uaf test, which covers the fault
> path. Its page is gone before the first access, so it faults on the load;
> the CMPXCHG entry is only reached when the page goes away between the two
> accesses, which a test cannot arrange.
>
> Finally cover fetching AND/OR/XOR through a number in
> verifier_arena_scalar, on x86-64 and arm64. Both widths are run with the
> operand in a register and in R0, which is where the x86-64 JIT keeps the
> operand on the stack because BPF_REG_AX holds the address. The 32-bit
> operands have bits set above 32 so that a missing zero-extension shows.
> The page is also freed first, after storing a nonzero value in it so that
> an access which did not fault would show, and the test checks that the
> fetched register comes back 0 and R0 is left alone, and that reading the
> address back gives 0. R0 is compared with a value derived from the page
> address: the verifier would decide a comparison with a constant at load
> time and drop it.
>
> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun