Skip to content

Commit ab6c637

Browse files
Yonghong SongAlexei Starovoitov
authored andcommitted
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: Alexei Starovoitov <[email protected]> Fixes: 738c96d ("bpf: Allow local kptrs to be exchanged via bpf_kptr_xchg") Signed-off-by: Yonghong Song <[email protected]> Acked-by: Kumar Kartikeya Dwivedi <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Alexei Starovoitov <[email protected]>
1 parent d565183 commit ab6c637

File tree

1 file changed

+15
-10
lines changed

1 file changed

+15
-10
lines changed

kernel/bpf/verifier.c

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4990,20 +4990,22 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
49904990
struct bpf_reg_state *reg, u32 regno)
49914991
{
49924992
const char *targ_name = btf_type_name(kptr_field->kptr.btf, kptr_field->kptr.btf_id);
4993-
int perm_flags = PTR_MAYBE_NULL | PTR_TRUSTED | MEM_RCU;
4993+
int perm_flags;
49944994
const char *reg_name = "";
49954995

4996-
/* Only unreferenced case accepts untrusted pointers */
4997-
if (kptr_field->type == BPF_KPTR_UNREF)
4998-
perm_flags |= PTR_UNTRUSTED;
4996+
if (btf_is_kernel(reg->btf)) {
4997+
perm_flags = PTR_MAYBE_NULL | PTR_TRUSTED | MEM_RCU;
4998+
4999+
/* Only unreferenced case accepts untrusted pointers */
5000+
if (kptr_field->type == BPF_KPTR_UNREF)
5001+
perm_flags |= PTR_UNTRUSTED;
5002+
} else {
5003+
perm_flags = PTR_MAYBE_NULL | MEM_ALLOC;
5004+
}
49995005

50005006
if (base_type(reg->type) != PTR_TO_BTF_ID || (type_flag(reg->type) & ~perm_flags))
50015007
goto bad_type;
50025008

5003-
if (!btf_is_kernel(reg->btf)) {
5004-
verbose(env, "R%d must point to kernel BTF\n", regno);
5005-
return -EINVAL;
5006-
}
50075009
/* We need to verify reg->type and reg->btf, before accessing reg->btf */
50085010
reg_name = btf_type_name(reg->btf, reg->btf_id);
50095011

@@ -5016,7 +5018,7 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
50165018
if (__check_ptr_off_reg(env, reg, regno, true))
50175019
return -EACCES;
50185020

5019-
/* A full type match is needed, as BTF can be vmlinux or module BTF, and
5021+
/* A full type match is needed, as BTF can be vmlinux, module or prog BTF, and
50205022
* we also need to take into account the reg->off.
50215023
*
50225024
* We want to support cases like:
@@ -7916,7 +7918,10 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
79167918
verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");
79177919
return -EFAULT;
79187920
}
7919-
/* Handled by helper specific checks */
7921+
if (meta->func_id == BPF_FUNC_kptr_xchg) {
7922+
if (map_kptr_match_type(env, meta->kptr_field, reg, regno))
7923+
return -EACCES;
7924+
}
79207925
break;
79217926
case PTR_TO_BTF_ID | MEM_PERCPU:
79227927
case PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED:

0 commit comments

Comments
 (0)