[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