Commit 723ab605 authored by Alexei Starovoitov's avatar Alexei Starovoitov Committed by Tengda Wu
Browse files

bpf: Fix verifier assumptions about socket->sk 

mainline inclusion
from mainline-v6.10-rc1
commit 0db63c0b86e981a1e97d2596d64ceceba1a5470e
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IA77YV
CVE: CVE-2024-38566

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0db63c0b86e981a1e97d2596d64ceceba1a5470e



--------------------------------

The verifier assumes that 'sk' field in 'struct socket' is valid
and non-NULL when 'socket' pointer itself is trusted and non-NULL.
That may not be the case when socket was just created and
passed to LSM socket_accept hook.
Fix this verifier assumption and adjust tests.

Reported-by: default avatarLiam Wisehart <liamwisehart@meta.com>
Acked-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
Fixes: 6fcd486b ("bpf: Refactor RCU enforcement in the verifier.")
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/r/20240427002544.68803-1-alexei.starovoitov@gmail.com


Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>

Conflicts:
        kernel/bpf/verifier.c
[this is because we did not backport commit 36d8bdf75a93 ("bpf: Add
alloc/xchg/direct_access support for local percpu kptr")] since it
is a feature commit]
Signed-off-by: default avatarTengda Wu <wutengda2@huawei.com>
parent 6fb08040

kernel/bpf/verifier.c

+18 −5
Original line number Diff line number Diff line
@@ -2543,6 +2543,8 @@ static void mark_btf_ld_reg(struct bpf_verifier_env *env, 
	regs[regno].type = PTR_TO_BTF_ID | flag;

	regs[regno].btf = btf;

	regs[regno].btf_id = btf_id;

	if (type_may_be_null(flag))

		regs[regno].id = ++env->id_gen;

}









#define DEF_NOT_SUBREG	(0)








@@ -5375,8 +5377,6 @@ static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,







				rcu_safe_kptr(kptr_field) && in_rcu_cs(env) ?









				PTR_MAYBE_NULL | MEM_RCU :









				PTR_MAYBE_NULL | PTR_UNTRUSTED);









		/* For mark_ptr_or_null_reg */









		val_reg->id = ++env->id_gen;









	} else if (class == BPF_STX) {









		val_reg = reg_state(env, value_regno);









		if (!register_is_null(val_reg) &&








@@ -5686,7 +5686,8 @@ static bool is_trusted_reg(const struct bpf_reg_state *reg)







		return true;

















	/* Types listed in the reg2btf_ids are always trusted */









	if (reg2btf_ids[base_type(reg->type)])









	if (reg2btf_ids[base_type(reg->type)] &&









	    !bpf_type_has_unsafe_modifiers(reg->type))









		return true;

















	/* If a register is not referenced, it is trusted if it has the








@@ -6265,6 +6266,7 @@ static int bpf_map_direct_read(struct bpf_map *map, int off, int size, u64 *val,







#define BTF_TYPE_SAFE_RCU(__type)  __PASTE(__type, __safe_rcu)









#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)

















/*









 * Allow list few fields as RCU trusted or full trusted.








@@ -6328,7 +6330,7 @@ BTF_TYPE_SAFE_TRUSTED(struct dentry) {







	struct inode *d_inode;









};

















BTF_TYPE_SAFE_TRUSTED(struct socket) {









BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket) {









	struct sock *sk;









};
















@@ -6363,11 +6365,20 @@ static bool type_is_trusted(struct bpf_verifier_env *env,







	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct linux_binprm));









	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct file));









	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct dentry));









	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED(struct socket));

















	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id, "__safe_trusted");









}

















static bool type_is_trusted_or_null(struct bpf_verifier_env *env,









				    struct bpf_reg_state *reg,









				    const char *field_name, u32 btf_id)









{









	BTF_TYPE_EMIT(BTF_TYPE_SAFE_TRUSTED_OR_NULL(struct socket));

















	return btf_nested_type_is_trusted(&env->log, reg, field_name, btf_id,









					  "__safe_trusted_or_null");









}

















static int check_ptr_to_btf_access(struct bpf_verifier_env *env,









				   struct bpf_reg_state *regs,









				   int regno, int off, int size,








@@ -6476,6 +6487,8 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,







		 */









		if (type_is_trusted(env, reg, field_name, btf_id)) {









			flag |= PTR_TRUSTED;









		} else if (type_is_trusted_or_null(env, reg, field_name, btf_id)) {









			flag |= PTR_TRUSTED | PTR_MAYBE_NULL;









		} else if (in_rcu_cs(env) && !type_may_be_null(reg->type)) {









			if (type_is_rcu(env, reg, field_name, btf_id)) {









				/* ignore __rcu tag and mark it MEM_RCU */
































tools/testing/selftests/bpf/progs/bench_local_storage_create.c



+3
−2






























Original line number
Diff line number
Diff line







@@ -61,14 +61,15 @@ SEC("lsm.s/socket_post_create")







int BPF_PROG(socket_post_create, struct socket *sock, int family, int type,









	     int protocol, int kern)









{









	struct sock *sk = sock->sk;









	struct storage *stg;









	__u32 pid;



















	pid = bpf_get_current_pid_tgid() >> 32;









	if (pid != bench_pid)









	if (pid != bench_pid || !sk)









		return 0;



















	stg = bpf_sk_storage_get(&sk_storage_map, sock->sk, NULL,









	stg = bpf_sk_storage_get(&sk_storage_map, sk, NULL,









				 BPF_LOCAL_STORAGE_GET_F_CREATE);



















	if (stg)
































tools/testing/selftests/bpf/progs/local_storage.c



+11
−9






























Original line number
Diff line number
Diff line







@@ -140,11 +140,12 @@ int BPF_PROG(socket_bind, struct socket *sock, struct sockaddr *address,







{









	__u32 pid = bpf_get_current_pid_tgid() >> 32;









	struct local_storage *storage;









	struct sock *sk = sock->sk;



















	if (pid != monitored_pid)









	if (pid != monitored_pid || !sk)









		return 0;



















	storage = bpf_sk_storage_get(&sk_storage_map, sock->sk, 0, 0);









	storage = bpf_sk_storage_get(&sk_storage_map, sk, 0, 0);









	if (!storage)









		return 0;


















@@ -155,24 +156,24 @@ int BPF_PROG(socket_bind, struct socket *sock, struct sockaddr *address,







	/* This tests that we can associate multiple elements









	 * with the local storage.









	 */









	storage = bpf_sk_storage_get(&sk_storage_map2, sock->sk, 0,









	storage = bpf_sk_storage_get(&sk_storage_map2, sk, 0,









				     BPF_LOCAL_STORAGE_GET_F_CREATE);









	if (!storage)









		return 0;



















	if (bpf_sk_storage_delete(&sk_storage_map2, sock->sk))









	if (bpf_sk_storage_delete(&sk_storage_map2, sk))









		return 0;



















	storage = bpf_sk_storage_get(&sk_storage_map2, sock->sk, 0,









	storage = bpf_sk_storage_get(&sk_storage_map2, sk, 0,









				     BPF_LOCAL_STORAGE_GET_F_CREATE);









	if (!storage)









		return 0;



















	if (bpf_sk_storage_delete(&sk_storage_map, sock->sk))









	if (bpf_sk_storage_delete(&sk_storage_map, sk))









		return 0;



















	/* Ensure that the sk_storage_map is disconnected from the storage. */









	if (!sock->sk->sk_bpf_storage || sock->sk->sk_bpf_storage->smap)









	if (!sk->sk_bpf_storage || sk->sk_bpf_storage->smap)









		return 0;



















	sk_storage_result = 0;








@@ -185,11 +186,12 @@ int BPF_PROG(socket_post_create, struct socket *sock, int family, int type,







{









	__u32 pid = bpf_get_current_pid_tgid() >> 32;









	struct local_storage *storage;









	struct sock *sk = sock->sk;



















	if (pid != monitored_pid)









	if (pid != monitored_pid || !sk)









		return 0;



















	storage = bpf_sk_storage_get(&sk_storage_map, sock->sk, 0,









	storage = bpf_sk_storage_get(&sk_storage_map, sk, 0,









				     BPF_LOCAL_STORAGE_GET_F_CREATE);









	if (!storage)









		return 0;
































tools/testing/selftests/bpf/progs/lsm_cgroup.c



+6
−2






























Original line number
Diff line number
Diff line







@@ -103,11 +103,15 @@ static __always_inline int real_bind(struct socket *sock,







				     int addrlen)









{









	struct sockaddr_ll sa = {};









	struct sock *sk = sock->sk;



















	if (sock->sk->__sk_common.skc_family != AF_PACKET)









	if (!sk)









		return 1;



















	if (sk->__sk_common.skc_family != AF_PACKET)









		return 1;



















	if (sock->sk->sk_kern_sock)









	if (sk->sk_kern_sock)









		return 1;



















	bpf_probe_read_kernel(&sa, sizeof(sa), address);