[PATCH bpf-next 1/2] bpf: Check load-acquire src ptr type before the load

[Daniel Borkmann <daniel@iogearbox.net>]

check_atomic_load() calls check_load_mem() before atomic_ptr_type_ok().
For a load-acquire that fetches into its own source register (dst_reg ==
src_reg), check_load_mem() overwrites src_reg's type with the type of the
loaded value, so the subsequent atomic_ptr_type_ok() no longer sees the
source pointer and fails to reject the disallowed types (ctx, pkt,
flow_keys, sock).

Since bpf_convert_ctx_accesses() does not rewrite atomic loads, the raw
access to the underlying kernel object is left in place. The destination
type is taken from the ctx access itself, so a load-acquire of the sk
field of struct __sk_buff for example leaves the register typed as
PTR_TO_SOCK_COMMON_OR_NULL, which type_is_sk_pointer() does not match
either, while it actually holds unconverted struct sk_buff bytes. Once
the NULL check has passed this is a type confusion, not just a leak of
kernel data.

Validate src_reg with check_reg_arg() and check the source pointer type
with atomic_ptr_type_ok() before the load again, mirroring
check_atomic_rmw(). Out-of-range register numbers are already rejected
earlier by check_and_resolve_insns() (commit 503d21ef8eac ("bpf: Do
register range validation early")), and the only exemption there,
is_stack_arg_ldx(), requires BPF_LDX | BPF_MEM | BPF_DW and thus never
matches a BPF_ATOMIC insn. atomic_ptr_type_ok() can therefore not
dereference register state out of bounds, that is, the out-of-bounds
read addressed by the Fixes commit below does not reappear (as proven
also via selftest).

Fixes: c03bb2fa327e ("bpf: Fix out-of-bounds read in check_atomic_load/store()")
Reported-by: STAR Labs SG <info@starlabs.sg>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 kernel/bpf/verifier.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b274004fccfd..9513e18836c2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6616,7 +6616,7 @@ static int check_atomic_load(struct bpf_verifier_env *env,
 {
 	int err;
 
-	err = check_load_mem(env, insn, true, false, false, "atomic_load");
+	err = check_reg_arg(env, insn->src_reg, SRC_OP);
 	if (err)
 		return err;
 
@@ -6627,7 +6627,7 @@ static int check_atomic_load(struct bpf_verifier_env *env,
 		return -EACCES;
 	}
 
-	return 0;
+	return check_load_mem(env, insn, true, false, false, "atomic_load");
 }
 
 static int check_atomic_store(struct bpf_verifier_env *env,
-- 
2.43.0