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

Daniel Borkmann <[email protected]>
Newsgroups org.kernel.vger.bpf
Message-ID <[email protected]>
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 <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.