Michal Luczaj reported use-after-free of unix_sk(sk)->peer by bpf_skc_to_unix_sock(). [0] Accessing unix_sk(sk)->peer is safe only under unix_state_lock(), but there are many functions where bpf prog can access the field locklessly via fentry/fexit. unix_dgram_connect() could clear unix_sk(sk)->peer and release the last refcnt of the peer sk while a bpf prog is accessing it, resulting in use-after-free. Another problematic scenario is that unix_sk(sk)->peer could go away while being passed to bpf_setsockopt() in bpf iter. To avoid such issues, let's reject access to unix_sk(sk)->peer by marking the pointer with PTR_UNTRUSTED. If needed, we could add a new helper later that uses unix_peer_get() and requires bpf_sk_release(). [0]: BUG: KASAN: slab-use-after-free in bpf_skc_to_unix_sock+0xa4/0xb0 Read of size 2 at addr ffff888147d38890 by task test_progs/2495 Call Trace: dump_stack_lvl+0x5d/0x80 print_report+0x170/0x4f3 kasan_report+0xe1/0x180 bpf_skc_to_unix_sock+0xa4/0xb0 bpf_prog_564a1c39c35d86a2_unix_shutdown_entry+0x8a/0x8e bpf_trampoline_6442564662+0x47/0xab unix_shutdown+0x9/0x880 __sys_shutdown+0xe1/0x160 __x64_sys_shutdown+0x52/0x90 do_syscall_64+0x6b/0x3a0 entry_SYSCALL_64_after_hwframe+0x76/0x7e Fixes: 9eeb3aa33ae0 ("bpf: Add bpf_skc_to_unix_sock() helper") Reported-by: Michal Luczaj Closes: https://lore.kernel.org/all/408569e7-2b82-4eff-b767-79ce6ef6cae0@rbox.co/ Suggested-by: Martin KaFai Lau Signed-off-by: Kuniyuki Iwashima --- kernel/bpf/verifier.c | 18 +++++++++++++ .../selftests/bpf/progs/verifier_sock.c | 25 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 3135643d5695..b328a1640c82 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -7076,6 +7076,7 @@ static int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val, #define BTF_TYPE_SAFE_RCU_OR_NULL(__type) __PASTE(__type, __safe_rcu_or_null) #define BTF_TYPE_SAFE_TRUSTED(__type) __PASTE(__type, __safe_trusted) #define BTF_TYPE_SAFE_TRUSTED_OR_NULL(__type) __PASTE(__type, __safe_trusted_or_null) +#define BTF_TYPE_SAFE_UNTRUSTED(__type) __PASTE(__type, __safe_untrusted) /* * Allow list few fields as RCU trusted or full trusted. @@ -7154,6 +7155,10 @@ BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct vm_area_struct) { struct file *vm_file; }; +BTF_TYPE_SAFE_UNTRUSTED(struct unix_sock) { + struct sock *peer; +}; + static bool type_is_rcu(struct bpf_verifier_env *env, struct bpf_reg_state *reg, const char *field_name, u32 btf_id) @@ -7201,6 +7206,16 @@ static bool type_is_trusted_or_null(struct bpf_verifier_env *env, "__safe_trusted_or_null"); } +static bool type_is_untrusted(struct bpf_verifier_env *env, + struct bpf_reg_state *reg, + const char *field_name, u32 btf_id) +{ + BTF_TYPE_EMIT(BTF_TYPE_SAFE_UNTRUSTED(struct unix_sock)); + + return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, + "__safe_untrusted"); +} + static int check_ptr_to_btf_access(struct bpf_verifier_env *env, struct bpf_reg_state *regs, int regno, int off, int size, @@ -7343,6 +7358,9 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env, } else { /* Old compat. Deprecated */ clear_trusted_flags(&flag); + + if (type_is_untrusted(env, reg, field_name, btf_id)) + flag |= PTR_UNTRUSTED; } if (atype == BPF_READ && value_regno >= 0) { diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c index a2132c72d3b8..8de4d3ed98d4 100644 --- a/tools/testing/selftests/bpf/progs/verifier_sock.c +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c @@ -3,6 +3,7 @@ #include "vmlinux.h" #include +#include #include "bpf_misc.h" struct { @@ -1166,4 +1167,28 @@ int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk) return TCX_PASS; } +SEC("fentry/unix_dgram_sendmsg") +__failure __msg("R1 type=untrusted_ptr_ expected=sock_common, sock, tcp_sock, xdp_sock, ptr_, trusted_ptr_") +int BPF_PROG(trace_unix_dgram_sendmsg, struct socket *sock, struct msghdr *msg, + size_t len) +{ + struct unix_sock *u, *u_other; + + if (!sock) + return 0; + + u = bpf_skc_to_unix_sock(sock->sk); + if (!u) + return 0; + + /* unix_dgram_connect() could clear u->peer + * and the peer could be freed. + */ + u_other = bpf_skc_to_unix_sock(u->peer); + if (!u_other) + return 0; + + return 0; +} + char _license[] SEC("license") = "GPL"; -- 2.53.0.rc2.204.g2597b5adb4-goog