[PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection

[PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection

[Daniel Borkmann]

A BPF_LOAD_ACQ is not rewritten to a BPF_PROBE_MEM load by the verifier,
unlike a regular BPF_LDX, so the JIT emits a plain load with no exception
table entry and a fault panics the kernel instead of being handled.

Reject the source pointer types that a BPF_LDX would have had that fault
protection applied to, i.e. the ones bpf_convert_ctx_accesses() turns
into BPF_PROBE_MEM: a bare PTR_TO_BTF_ID, PTR_TO_BTF_ID | PTR_UNTRUSTED,
PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED and PTR_TO_MEM | MEM_RDONLY |
PTR_UNTRUSTED.

This is reachable e.g. by loading ->mm out of a trusted task_struct
yields an untrusted pointer to mm_struct, and it is NULL for a kernel
thread:

  [...]
  SEC("tp_btf/sched_switch")
  int BPF_PROG(demo, bool preempt, struct task_struct *prev,
               struct task_struct *next)
  {
      struct mm_struct *mm = next->mm;  /* untrusted */

      out_ldx = (__u64)mm->pgd;         /* BPF_LDX      */
      out_acq = load_acquire(&mm->pgd); /* BPF_LOAD_ACQ */
      return 0;
  }
  [...]

Both dereference the same pointer, but only the BPF_LDX is protected
(x86-64 JIT, jump targets shown prog-relative):

  [...]
  ; out_ldx = (__u64)mm->pgd;
  17:   movq    $-10485760, %r10
  1e:   movq    %rsi, %r11
  21:   addq    $184, %r11
  28:   subq    %r10, %r11
  2b:   movabsq $140737498841088, %r10
  35:   cmpq    %r10, %r11
  38:   ja      0x3e                 <-- kernel addr?
  3a:   xorl    %edi, %edi           <-- no: dst = 0, skip the load
  3c:   jmp     0x45
  3e:   movq    184(%rsi), %rdi      <-- yes: load + extable entry
  [...]
  ; load_acquire(&mm->pgd)
  53:	movq    %rsi, %rdi
  56:	movq    184(%rdi), %rax       <-- no check, no extable entry
  [...]

Note that BPF_PROBE_MEM is not visible in a bpftool xlated dump, as
bpf_insn_prepare_dump() rewrites it back to BPF_MEM.

A PTR_TRUSTED pointer is deliberately not on the list. Such a load is
not converted either, but it does not need to be, since the pointer is
guaranteed live, so load-acquire from it stays allowed.

The check is gated on BPF_LOAD_ACQ so that atomic RMW and store-release
error messages are unchanged; writes (RMW / store-release) to such
pointers are already rejected elsewhere, so only load-acquire needs this.

Fixes: 880442305a39 ("bpf: Introduce load-acquire and store-release instructions")
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/verifier.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e6233c0081d1..648c5784178e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4923,6 +4923,30 @@ static bool is_arena_reg(struct bpf_verifier_env *env, int regno)
 	return reg->type == PTR_TO_ARENA;
 }
 
+static bool is_load_acq_unsafe(struct bpf_verifier_env *env, int regno,
+			       struct bpf_insn *insn)
+{
+	const struct bpf_reg_state *reg = reg_state(env, regno);
+
+	/*
+	 * A BPF_LOAD_ACQ is not rewritten to a BPF_PROBE_MEM load by the
+	 * verifier, unlike a regular BPF_LDX. The JIT would emit a plain load
+	 * with no exception table entry, so a fault (e.g. NULL deref) crashes
+	 * the kernel instead of being handled.
+	 *
+	 * Reject the source pointer types that a BPF_LDX would have had that
+	 * fault protection applied to, i.e. the ones bpf_convert_ctx_accesses()
+	 * turns into BPF_PROBE_MEM: a bare PTR_TO_BTF_ID and any PTR_UNTRUSTED
+	 * pointer (untrusted btf ids, untrusted MEM_ALLOC, rdonly untrusted
+	 * memory). A PTR_TRUSTED pointer is not among them, is not converted,
+	 * and stays allowed. Same for the other flagged PTR_TO_BTF_ID variants
+	 * (MEM_ALLOC, MEM_RCU, ...), hence the exact match on the base type.
+	 */
+	return insn->imm == BPF_LOAD_ACQ &&
+	       (reg->type == PTR_TO_BTF_ID ||
+		(type_flag(reg->type) & PTR_UNTRUSTED));
+}
+
 /* Return false if @regno contains a pointer whose type isn't supported for
  * atomic instruction @insn.
  */
@@ -4939,7 +4963,8 @@ static bool atomic_ptr_type_ok(struct bpf_verifier_env *env, int regno,
 		return false;
 	if (is_arena_reg(env, regno))
 		return bpf_jit_supports_insn(insn, true);
-
+	if (is_load_acq_unsafe(env, regno, insn))
+		return false;
 	return true;
 }
 
-- 
2.43.0

[PATCH bpf-next v2 2/6] bpf, riscv: Add and use bpf_atomic_is_load_acq() helper

[Daniel Borkmann]

A load-acquire is the only BPF_STX class instruction that reads from
src_reg into dst_reg, that is, it has the operand roles of a BPF_LDX.
JIT code which tells loads from stores apart by instruction class alone
has to special case it, for example when deciding which register holds
the faulting address and which one to clear from an exception handler.

riscv64 already does so, open coded as a bare insn->imm test. Add a
bpf_atomic_is_load_acq() helper and convert riscv64 over to it, so that
the x86-64 and arm64 JITs can use the same helper in subsequent patches.

Unlike bpf_atomic_is_load_store(), which presumes that its argument is
already known to be a BPF_ATOMIC instruction, the new helper is called
from code which still sees all instruction classes, so it checks class
and mode itself.

Also, move bpf_atomic_is_load_store() to filter.h next to BPF_ATOMIC_OP,
so that both helpers stay together. No functional change intended.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 arch/riscv/net/bpf_jit_comp64.c |  2 +-
 include/linux/bpf.h             | 15 ---------------
 include/linux/filter.h          | 31 +++++++++++++++++++++++++++++++
 3 files changed, 32 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 8fe8969fb8a0..6b9972b07c1b 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1994,7 +1994,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 		/* ret can be 1 (skip-zext); extable entry still needs to be added */
 		if (ret >= 0)
 			ret = add_exception_handler(insn,
-				insn->imm == BPF_LOAD_ACQ ? rd : REG_DONT_CLEAR_MARKER,
+				bpf_atomic_is_load_acq(insn) ? rd : REG_DONT_CLEAR_MARKER,
 				ctx) ?: ret;
 
 		if (ret)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 73bacfc6444d..d79bf7557ef6 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1132,21 +1132,6 @@ static inline bool bpf_pseudo_func(const struct bpf_insn *insn)
 	return bpf_is_ldimm64(insn) && insn->src_reg == BPF_PSEUDO_FUNC;
 }
 
-/* Given a BPF_ATOMIC instruction @atomic_insn, return true if it is an
- * atomic load or store, and false if it is a read-modify-write instruction.
- */
-static inline bool
-bpf_atomic_is_load_store(const struct bpf_insn *atomic_insn)
-{
-	switch (atomic_insn->imm) {
-	case BPF_LOAD_ACQ:
-	case BPF_STORE_REL:
-		return true;
-	default:
-		return false;
-	}
-}
-
 struct bpf_prog_ops {
 	int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr,
 			union bpf_attr __user *uattr);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 32d5297c557e..41b02d53e222 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -383,6 +383,37 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
 /* Legacy alias */
 #define BPF_STX_XADD(SIZE, DST, SRC, OFF) BPF_ATOMIC_OP(SIZE, BPF_ADD, DST, SRC, OFF)
 
+/*
+ * Given a BPF_ATOMIC instruction @atomic_insn, return true if it is an
+ * atomic load or store, and false if it is a read-modify-write instruction.
+ */
+static inline bool
+bpf_atomic_is_load_store(const struct bpf_insn *atomic_insn)
+{
+	switch (atomic_insn->imm) {
+	case BPF_LOAD_ACQ:
+	case BPF_STORE_REL:
+		return true;
+	default:
+		return false;
+	}
+}
+
+/*
+ * A load-acquire is the only BPF_STX class instruction that reads into
+ * dst_reg from src_reg + off16, i.e. it has the operand roles of a BPF_LDX.
+ * Unlike bpf_atomic_is_load_store(), @insn is not assumed to be a BPF_ATOMIC
+ * instruction here, so that callers which walk all instruction classes can
+ * use this directly.
+ */
+static inline bool bpf_atomic_is_load_acq(const struct bpf_insn *insn)
+{
+	return BPF_CLASS(insn->code) == BPF_STX &&
+	       (BPF_MODE(insn->code) == BPF_ATOMIC ||
+		BPF_MODE(insn->code) == BPF_PROBE_ATOMIC) &&
+	       insn->imm == BPF_LOAD_ACQ;
+}
+
 /* Memory store, *(uint *) (dst_reg + off16) = imm32 */
 
 #define BPF_ST_MEM(SIZE, DST, OFF, IMM)				\
-- 
2.43.0

[PATCH bpf-next v2 3/6] bpf, x86: Fix exception table metadata for arena load-acquire

[Daniel Borkmann]

A load-acquire from an arena pointer is converted to BPF_PROBE_ATOMIC and
gets an exception table entry, but the entry is filled in as if it were a
store, since populate_extable() decides based on instruction class alone
and a load-acquire is of BPF_STX class:

  if (BPF_CLASS(insn->code) == BPF_LDX) {
      arena_reg = reg2pt_regs[src_reg];
      fixup_reg = reg2pt_regs[dst_reg];
  } else {
      arena_reg = reg2pt_regs[dst_reg];
      fixup_reg = DONT_CLEAR;
  }

For a load-acquire dst_reg holds the loaded value and src_reg holds the
address, so both assignments in the else branch are wrong. On a fault
over an unmapped arena page ex_handler_bpf() then:

  - computes the reported address from the value register instead
    of the address register
  - reports the access as a WRITE, since it derives the direction
    from fixup_reg == DONT_CLEAR
  - leaves dst_reg untouched, so the program continues with a stale
    value instead of the 0 that BPF_PROBE_* loads deliver

The access itself is emitted correctly, emit_atomic_ld_st_index() uses
src_reg as the address, so this is a broken probe contract and a wrong
diagnostic rather than a memory safety issue.

Use bpf_atomic_is_load_acq() helper so a load-acquire takes the load path.

Fixes: 5341c9a4d833 ("bpf, x86: Support load-acquire and store-release instructions")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Peilin Ye <yepeilin@google.com>
---
 arch/x86/net/bpf_jit_comp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 01e7ce569c1e..88ed95b2eaa7 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -2331,8 +2331,13 @@ st:			insn_off = insn->off;
 				 * BPF_PROBE_ATOMIC) before being used for the memory access. Pass
 				 * the reg holding the unmodified 32-bit address to
 				 * ex_handler_bpf().
+				 *
+				 * 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.
 				 */
-				if (BPF_CLASS(insn->code) == BPF_LDX) {
+				if (BPF_CLASS(insn->code) == BPF_LDX ||
+				    bpf_atomic_is_load_acq(insn)) {
 					arena_reg = reg2pt_regs[src_reg];
 					fixup_reg = reg2pt_regs[dst_reg];
 				} else {
-- 
2.43.0

[PATCH bpf-next v2 4/6] bpf, arm64: Fix exception table metadata for arena load-acquire

[Daniel Borkmann]

Same problem as on x86-64: add_exception_handler() decides whether an
instruction is a load by its class, and a load-acquire is of BPF_STX
class even though it reads from src_reg into dst_reg. As a result ...

  if (BPF_CLASS(insn->code) != BPF_LDX)
          dst_reg = DONT_CLEAR;

... drops the register to clear, and ...

  if (BPF_CLASS(insn->code) == BPF_LDX)
          arena_reg = bpf2a64[insn->src_reg];
  else
          arena_reg = bpf2a64[insn->dst_reg];

... hands ex_handler_bpf() the value register instead of the address
register. A load-acquire from an arena pointer that faults on an
unmapped page is therefore reported as a WRITE at a bogus address,
and dst_reg keeps its previous value instead of being cleared to 0.

Note that emit_atomic_ld_st() already picks src_reg as the address
for BPF_LOAD_ACQ, so only the exception table metadata was out of sync
with the emitted access.

Same as on x86-64, use bpf_atomic_is_load_acq() so a load-acquire takes
the load path.

Fixes: 9bb12368d539 ("bpf, arm64: Support load-acquire and store-release instructions")
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
Cc: Peilin Ye <yepeilin@google.com>
---
 arch/arm64/net/bpf_jit_comp.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 4cdc7dfb05ba..d14d297ebb96 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -1178,7 +1178,12 @@ static int add_exception_handler(const struct bpf_insn *insn,
 
 	ex->insn = ins_offset;
 
-	if (BPF_CLASS(insn->code) != BPF_LDX)
+	/*
+	 * 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.
+	 */
+	if (BPF_CLASS(insn->code) != BPF_LDX && !bpf_atomic_is_load_acq(insn))
 		dst_reg = DONT_CLEAR;
 
 	ex->fixup = FIELD_PREP(BPF_FIXUP_REG_MASK, dst_reg);
@@ -1193,7 +1198,7 @@ static int add_exception_handler(const struct bpf_insn *insn,
 		 * memory access. Pass the reg holding the unmodified 32-bit address to
 		 * ex_handler_bpf.
 		 */
-		if (BPF_CLASS(insn->code) == BPF_LDX)
+		if (BPF_CLASS(insn->code) == BPF_LDX || bpf_atomic_is_load_acq(insn))
 			arena_reg = bpf2a64[insn->src_reg];
 		else
 			arena_reg = bpf2a64[insn->dst_reg];
-- 
2.43.0

[PATCH bpf-next v2 5/6] selftests/bpf: Add arena fault test for load-acquire

[Daniel Borkmann]

Add stream_arena_load_acquire_fault, which performs a load-acquire from an
unmapped arena address, next to the existing read and write fault tests.

The test covers both halves of the JIT bug that treated a load-acquire as
a store when populating its exception table entry:

  - the fault has to be reported as a READ, and at the address held by
    the source register, which __stderr() and test_address() check, and
  - the destination register has to be cleared by the fault handler,
    which the program checks by poisoning it before the load-acquire
    and returning it, so __retval(0) fails if it is left untouched

Note, load-acquire is 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_arena_fault_address
  [...]
  #462/1   stream_arena_fault_address/read_fault:OK
  #462/2   stream_arena_fault_address/write_fault:OK
  #462/3   stream_arena_fault_address/load_acquire_fault:OK
  #462     stream_arena_fault_address:OK
  Summary: 1/3 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 v1->v2:
   - Fix encoding via load_acquire_insn for s390

 .../testing/selftests/bpf/prog_tests/stream.c |  2 +
 tools/testing/selftests/bpf/progs/stream.c    | 44 +++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/stream.c b/tools/testing/selftests/bpf/prog_tests/stream.c
index c3cce5c292bd..15dd3ae2a84b 100644
--- a/tools/testing/selftests/bpf/prog_tests/stream.c
+++ b/tools/testing/selftests/bpf/prog_tests/stream.c
@@ -103,6 +103,8 @@ void test_stream_arena_fault_address(void)
 		test_address(skel->progs.stream_arena_read_fault, &skel->bss->fault_addr);
 	if (test__start_subtest("write_fault"))
 		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);
 
 	stream__destroy(skel);
 }
diff --git a/tools/testing/selftests/bpf/progs/stream.c b/tools/testing/selftests/bpf/progs/stream.c
index 8d8e53d37266..cf5533e11f39 100644
--- a/tools/testing/selftests/bpf/progs/stream.c
+++ b/tools/testing/selftests/bpf/progs/stream.c
@@ -185,6 +185,50 @@ int stream_arena_read_fault(void *ctx)
 	return 0;
 }
 
+SEC("syscall")
+__arch_x86_64
+__arch_arm64
+__success __retval(0)
+__stderr("ERROR: Arena READ 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_load_acquire_fault(void *ctx)
+{
+	static const struct bpf_insn load_acquire_insn = {
+		.code	 = 0xc3,	/* BPF_STX | BPF_ATOMIC | BPF_W */
+		.dst_reg = 0,		/* BPF_REG_0 */
+		.src_reg = 1,		/* BPF_REG_1 */
+		.off	 = 0x7fff,
+		.imm	 = 0x100,	/* BPF_LOAD_ACQ */
+	};
+	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);
+	asm volatile (
+		"r1 = %[user_vm_start];"
+		"r0 = 1;"
+		".8byte %[load_acquire_insn];" /* r0 = load_acquire((u32 *)(r1 + 0x7fff)) */
+		"%[val] = r0;"
+		: [val] "=r" (val)
+		: [user_vm_start] "r" (user_vm_start),
+		  __imm_insn(load_acquire_insn, load_acquire_insn)
+		: "r0", "r1"
+	);
+	return val;
+}
+
 static __noinline void subprog(void)
 {
 	int __arena *addr = (int __arena *)0xdeadbeef;
-- 
2.43.0

[PATCH bpf-next v2 6/6] selftests/bpf: Add load-acquire test for probe-memory pointer types

[Daniel Borkmann]

Add a verifier test that a BPF_LOAD_ACQ from a rdonly_untrusted_mem pointer
(PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED, obtained via bpf_rdonly_cast())
is rejected. Such a source requires BPF_PROBE_MEM fault protection which
is not applied to atomic loads; without the verifier fix the load is accepted
and would crash the kernel on a fault.

  # LDLIBS=-static PKG_CONFIG='pkg-config --static' ./vmtest.sh -- ./test_progs -t verifier_load_acquire
  [...]
  #621/1   verifier_load_acquire/load-acquire, 8-bit:OK
  #621/2   verifier_load_acquire/load-acquire, 8-bit @unpriv:OK
  #621/3   verifier_load_acquire/load-acquire, 16-bit:OK
  #621/4   verifier_load_acquire/load-acquire, 16-bit @unpriv:OK
  #621/5   verifier_load_acquire/load-acquire, 32-bit:OK
  #621/6   verifier_load_acquire/load-acquire, 32-bit @unpriv:OK
  #621/7   verifier_load_acquire/load-acquire, 64-bit:OK
  #621/8   verifier_load_acquire/load-acquire, 64-bit @unpriv:OK
  [...]
  #621/19  verifier_load_acquire/load-acquire from rdonly_untrusted_mem pointer:OK
  #621/20  verifier_load_acquire/load-acquire with invalid register R15:OK
  #621/21  verifier_load_acquire/load-acquire with invalid register R15 @unpriv:OK
  #621/22  verifier_load_acquire/load-acquire from pkt pointer:OK
  #621/23  verifier_load_acquire/load-acquire from flow_keys pointer:OK
  #621/24  verifier_load_acquire/load-acquire from sock pointer:OK
  #621     verifier_load_acquire:OK
  Summary: 1/24 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 .../bpf/progs/verifier_load_acquire.c         | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
index ae1dab1b0cbb..d17026d7480d 100644
--- a/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
+++ b/tools/testing/selftests/bpf/progs/verifier_load_acquire.c
@@ -3,6 +3,7 @@
 
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
 #include "../../../include/linux/filter.h"
 #include "bpf_misc.h"
 
@@ -221,6 +222,33 @@ __naked void load_acquire_from_sock_pointer(void)
 	: __clobber_all);
 }
 
+SEC("socket")
+__description("load-acquire from rdonly_untrusted_mem pointer")
+__failure __msg("BPF_ATOMIC loads from R{{[0-9]+}} rdonly_untrusted_mem is not allowed")
+int load_acquire_from_rdonly_untrusted_mem(void *ctx)
+{
+	__u64 val = 0;
+	void *p;
+
+	/*
+	 * bpf_rdonly_cast(x, 0) yields PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED.
+	 * A regular BPF_LDX from it is rewritten to BPF_PROBE_MEM, but a
+	 * load-acquire is not, so it must be rejected, otherwise the JIT emits
+	 * a plain load with no exception table entry and a fault would crash
+	 * the kernel.
+	 */
+	p = bpf_rdonly_cast(&val, 0);
+	asm volatile (
+	"r1 = %[p];"
+	".8byte %[load_acquire_insn];" // r0 = load_acquire((u64 *)(r1 + 0));
+	:
+	: [p] "r" (p),
+	  __imm_insn(load_acquire_insn,
+		     BPF_ATOMIC_OP(BPF_DW, BPF_LOAD_ACQ, BPF_REG_0, BPF_REG_1, 0))
+	: "r0", "r1");
+	return 0;
+}
+
 SEC("socket")
 __description("load-acquire with invalid register R15")
 __failure __failure_unpriv __msg("R15 is invalid")
-- 
2.43.0

Re: [PATCH bpf-next v2 3/6] bpf, x86: Fix exception table metadata for arena load-acquire

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The x86 JIT exception table metadata fix for `BPF_PROBE_ATOMIC` successfully addresses `BPF_LOAD_ACQ` but incorrectly handles read-modify-write (RMW) fetch operations (e.g., `BPF_CMPXCHG`, `BPF_XCHG`, `BPF_ADD | BPF_FETCH`), leading to silent lock acquisition successes and potential information leaks.
- [High] The ARM64 and s390 BPF JITs fail to properly populate exception table metadata for arena atomic instructions, leaving both `BPF_LOAD_ACQ` (on ARM64) and RMW fetch atomics (on both architectures) broken on unmapped page faults.
--

--- Patch [3]: [PATCH bpf-next v2 3/6] bpf, x86: Fix exception table metadata for arena load-acquire ---
commit 45e0ae10aa75994903c64f22211117edeb408932
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf, x86: Fix exception table metadata for arena load-acquire

This commit fixes an issue where load-acquire instructions on arena
pointers were incorrectly treated as stores by the exception table
logic on x86. It ensures the destination register is properly cleared
and the correct address register is reported during a fault.

> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 01e7ce569c1ed..88ed95b2eaa72 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -2331,8 +2331,13 @@ st:			insn_off = insn->off;
>  				 * BPF_PROBE_ATOMIC) before being used for the memory access. Pass
>  				 * the reg holding the unmodified 32-bit address to
>  				 * ex_handler_bpf().
> +				 *
> +				 * 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.
>  				 */
> -				if (BPF_CLASS(insn->code) == BPF_LDX) {
> +				if (BPF_CLASS(insn->code) == BPF_LDX ||
> +				    bpf_atomic_is_load_acq(insn)) {
>  					arena_reg = reg2pt_regs[src_reg];
>  					fixup_reg = reg2pt_regs[dst_reg];
>  				} else {
>  					arena_reg = reg2pt_regs[dst_reg];
>  					fixup_reg = DONT_CLEAR;
>  				}

[Severity: High]
This isn't a bug introduced by this patch, but it seems read-modify-write
(RMW) fetch operations like BPF_CMPXCHG, BPF_XCHG, and BPF_ADD | BPF_FETCH
might also be incorrectly handled here.

Since RMW fetch atomics belong to the BPF_STX class and are not LOAD_ACQ,
they incorrectly fall into this else branch in do_jit() where fixup_reg is
set to DONT_CLEAR.

If a BPF program executes a BPF_CMPXCHG on an unmapped arena page, the
exception handler will intercept the fault but leave the destination register
uncleared.

Could this allow a lock acquisition loop checking if the register matches the
expected value to falsely assume the lock was acquired, potentially bypassing
BPF concurrency controls?

Does this also risk leaking the uncleared pointer?


[Severity: High]
This is a pre-existing issue, but similar logic appears to be missing for
ARM64 and s390 JIT implementations for arena atomic instructions.

In arch/arm64/net/bpf_jit_comp.c:add_exception_handler():

	if (BPF_CLASS(insn->code) != BPF_LDX)
		dst_reg = DONT_CLEAR;

Does ARM64 also fail to identify BPF_LOAD_ACQ and RMW atomics as instructions
that require clearing a destination register, treating them as DONT_CLEAR?

This could trigger the exact same bug fixed for x86 in this patch.

Additionally, in arch/s390/net/bpf_jit_comp.c:bpf_jit_probe_atomic_pre():

	if (BPF_MODE(insn->code) != BPF_PROBE_ATOMIC)
		return;

For RMW atomics on s390, the function seems to fail to assign a register to be
cleared, meaning the exception handler does not clear any register upon an
unmapped page fault.

Could this also lead to silent lock acquisition bypass on s390?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806201047.333389-1-daniel@iogearbox.net?part=3

Re: [PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A pre-existing flaw allows merged untrusted pointer types to bypass BPF_PROBE_MEM rewriting for plain BPF_LDX instructions, leading to kernel panics upon NULL pointer dereference.
--

--- Patch [1]: [PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection ---
commit e15f6fca389dca527ccadeba23f22ab6d2f2f36d
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf: Reject load-acquire from pointers requiring fault protection

This commit prevents BPF_LOAD_ACQ instructions from using pointer types that
would normally require fault protection (like untrusted pointers or bare
PTR_TO_BTF_ID). Since BPF_LOAD_ACQ is not rewritten to use BPF_PROBE_MEM
by the verifier, a fault from these pointers would panic the kernel
instead of being safely handled. The fix explicitly rejects these unsafe
source pointer types for load-acquires.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e6233c0081d10..648c5784178ec 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4923,6 +4923,30 @@ static bool is_arena_reg(struct bpf_verifier_env *env, int regno)
>  	return reg->type == PTR_TO_ARENA;
>  }
>  
> +static bool is_load_acq_unsafe(struct bpf_verifier_env *env, int regno,
> +			       struct bpf_insn *insn)
> +{
> +	const struct bpf_reg_state *reg = reg_state(env, regno);
> +
> +	/*
> +	 * A BPF_LOAD_ACQ is not rewritten to a BPF_PROBE_MEM load by the
> +	 * verifier, unlike a regular BPF_LDX. The JIT would emit a plain load
> +	 * with no exception table entry, so a fault (e.g. NULL deref) crashes
> +	 * the kernel instead of being handled.
> +	 *
> +	 * Reject the source pointer types that a BPF_LDX would have had that
> +	 * fault protection applied to, i.e. the ones bpf_convert_ctx_accesses()
> +	 * turns into BPF_PROBE_MEM: a bare PTR_TO_BTF_ID and any PTR_UNTRUSTED
> +	 * pointer (untrusted btf ids, untrusted MEM_ALLOC, rdonly untrusted
> +	 * memory). A PTR_TRUSTED pointer is not among them, is not converted,
> +	 * and stays allowed. Same for the other flagged PTR_TO_BTF_ID variants
> +	 * (MEM_ALLOC, MEM_RCU, ...), hence the exact match on the base type.
> +	 */

[Severity: High]
This is a pre-existing issue, but does bpf_convert_ctx_accesses() actually
rewrite all PTR_UNTRUSTED pointers as the comment suggests?

It appears that if control flow paths merge at a BPF_LDX instruction,
save_aux_ptr_type() in kernel/bpf/verifier.c merges their pointer types:

    if (type & PTR_UNTRUSTED || *prev_type & PTR_UNTRUSTED)
        merged_type |= PTR_UNTRUSTED;
    if ((type & MEM_RDONLY) || (*prev_type & MEM_RDONLY))
        merged_type |= MEM_RDONLY;
    *prev_type = merged_type;

If one path uses a valid memory pointer like PTR_TO_MEM and the other uses
an untrusted pointer like PTR_TO_BTF_ID | PTR_UNTRUSTED, they merge into
PTR_TO_MEM | PTR_UNTRUSTED without the MEM_RDONLY flag.

However, looking at bpf_convert_ctx_accesses() in kernel/bpf/fixups.c:

    case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
    case PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED:
        if (type == BPF_READ) {
            if (BPF_MODE(insn->code) == BPF_MEM)

This switch statement doesn't have a case for PTR_TO_MEM | PTR_UNTRUSTED.
Could this allow a regular BPF_LDX instruction to bypass the BPF_PROBE_MEM
rewrite and cause a kernel panic if the untrusted pointer path is taken at
runtime and is NULL?

> +	return insn->imm == BPF_LOAD_ACQ &&
> +	       (reg->type == PTR_TO_BTF_ID ||
> +		(type_flag(reg->type) & PTR_UNTRUSTED));
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806201047.333389-1-daniel@iogearbox.net?part=1

Re: [PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Kumar Kartikeya Dwivedi <memxor@gmail.com>:

On Thu,  6 Aug 2026 22:10:42 +0200 you wrote:
> A BPF_LOAD_ACQ is not rewritten to a BPF_PROBE_MEM load by the verifier,
> unlike a regular BPF_LDX, so the JIT emits a plain load with no exception
> table entry and a fault panics the kernel instead of being handled.
> 
> Reject the source pointer types that a BPF_LDX would have had that fault
> protection applied to, i.e. the ones bpf_convert_ctx_accesses() turns
> into BPF_PROBE_MEM: a bare PTR_TO_BTF_ID, PTR_TO_BTF_ID | PTR_UNTRUSTED,
> PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED and PTR_TO_MEM | MEM_RDONLY |
> PTR_UNTRUSTED.
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2,1/6] bpf: Reject load-acquire from pointers requiring fault protection
    https://git.kernel.org/bpf/bpf-next/c/7db0a00445f1
  - [bpf-next,v2,2/6] bpf, riscv: Add and use bpf_atomic_is_load_acq() helper
    https://git.kernel.org/bpf/bpf-next/c/e2577cd62060
  - [bpf-next,v2,3/6] bpf, x86: Fix exception table metadata for arena load-acquire
    https://git.kernel.org/bpf/bpf-next/c/4cf8def58b77
  - [bpf-next,v2,4/6] bpf, arm64: Fix exception table metadata for arena load-acquire
    https://git.kernel.org/bpf/bpf-next/c/af22d273aa1f
  - [bpf-next,v2,5/6] selftests/bpf: Add arena fault test for load-acquire
    https://git.kernel.org/bpf/bpf-next/c/007466d9e497
  - [bpf-next,v2,6/6] selftests/bpf: Add load-acquire test for probe-memory pointer types
    https://git.kernel.org/bpf/bpf-next/c/2b1f9f69ae25

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html