| Message ID | 20230822050053.2886960-1-yonghong.song@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ab6c637ad0276e42f8acabcbc64932a6d346dab3 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v2,1/2] bpf: Fix a bpf_kptr_xchg() issue with local kptr | expand |
On Tue, 22 Aug 2023 at 10:31, Yonghong Song <yonghong.song@linux.dev> wrote: > > 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 <ast@kernel.org> > Fixes: 738c96d5e2e3 ("bpf: Allow local kptrs to be exchanged via bpf_kptr_xchg") > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- Acked-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Hello: This series was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Mon, 21 Aug 2023 22:00:53 -0700 you wrote: > 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 > > [...] Here is the summary with links: - [bpf-next,v2,1/2] bpf: Fix a bpf_kptr_xchg() issue with local kptr https://git.kernel.org/bpf/bpf-next/c/ab6c637ad027 - [bpf-next,v2,2/2] selftests/bpf: Add a failure test for bpf_kptr_xchg() with local kptr https://git.kernel.org/bpf/bpf-next/c/fb3015942643 You are awesome, thank you!
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 4ccca1f6c998..3a91bfd7b9cc 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -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 = ""; - /* Only unreferenced case accepts untrusted pointers */ - if (kptr_field->type == BPF_KPTR_UNREF) - perm_flags |= PTR_UNTRUSTED; + 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:
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 <ast@kernel.org> Fixes: 738c96d5e2e3 ("bpf: Allow local kptrs to be exchanged via bpf_kptr_xchg") Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- kernel/bpf/verifier.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) Changelogs: v1 -> v2: - call map_kptr_match_type() instead of btf_struct_ids_match().