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

[Eduard Zingerman <eddyz87@gmail.com>]

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.