Commit ab6c637a authored by Yonghong Song's avatar Yonghong Song Committed by Alexei Starovoitov
Browse files

bpf: Fix a bpf_kptr_xchg() issue with local kptr 

When reviewing local percpu kptr support, Alexei discovered a bug
wherea bpf_kptr_xchg() may succeed even if the map value kptr type and
locally allocated obj type do not match ([1]). Missed struct btf_id
comparison is the reason for the bug. This patch added such struct btf_id
comparison and will flag verification failure if types do not match.

  [1] https://lore.kernel.org/bpf/20230819002907.io3iphmnuk43xblu@macbook-pro-8.dhcp.thefacebook.com/#t



Reported-by: default avatarAlexei Starovoitov <ast@kernel.org>
Fixes: 738c96d5 ("bpf: Allow local kptrs to be exchanged via bpf_kptr_xchg")
Signed-off-by: default avatarYonghong Song <yonghong.song@linux.dev>
Acked-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20230822050053.2886960-1-yonghong.song@linux.dev


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent d5651838

kernel/bpf/verifier.c

+15 −10
Original line number Diff line number Diff line
@@ -4990,20 +4990,22 @@ static int map_kptr_match_type(struct bpf_verifier_env *env, 
			       struct bpf_reg_state *reg, u32 regno)

{

	const char *targ_name = btf_type_name(kptr_field->kptr.btf, kptr_field->kptr.btf_id);

	int perm_flags = PTR_MAYBE_NULL | PTR_TRUSTED | MEM_RCU;

	int perm_flags;

	const char *reg_name = "";









	if (btf_is_kernel(reg->btf)) {









		perm_flags = PTR_MAYBE_NULL | PTR_TRUSTED | MEM_RCU;

















		/* Only unreferenced case accepts untrusted pointers */









		if (kptr_field->type == BPF_KPTR_UNREF)









			perm_flags |= PTR_UNTRUSTED;









	} else {









		perm_flags = PTR_MAYBE_NULL | MEM_ALLOC;









	}

















	if (base_type(reg->type) != PTR_TO_BTF_ID || (type_flag(reg->type) & ~perm_flags))









		goto bad_type;

















	if (!btf_is_kernel(reg->btf)) {









		verbose(env, "R%d must point to kernel BTF\n", regno);









		return -EINVAL;









	}









	/* We need to verify reg->type and reg->btf, before accessing reg->btf */









	reg_name = btf_type_name(reg->btf, reg->btf_id);
















@@ -5016,7 +5018,7 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,







	if (__check_ptr_off_reg(env, reg, regno, true))









		return -EACCES;

















	/* A full type match is needed, as BTF can be vmlinux or module BTF, and









	/* A full type match is needed, as BTF can be vmlinux, module or prog BTF, and









	 * we also need to take into account the reg->off.









	 *









	 * We want to support cases like:








@@ -7916,7 +7918,10 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,







			verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");









			return -EFAULT;









		}









		/* Handled by helper specific checks */









		if (meta->func_id == BPF_FUNC_kptr_xchg) {









			if (map_kptr_match_type(env, meta->kptr_field, reg, regno))









				return -EACCES;









		}









		break;









	case PTR_TO_BTF_ID | MEM_PERCPU:









	case PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED: