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 <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>
Link: https://lore.kernel.org/r/20230822050053.2886960-1-yonghong.song@linux.dev
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 4ccca1f6c9981d8ade3fa4d07398b732068c0ede..3a91bfd7b9cce136d9adbb8ebe41588cb5c907cb 100644 (file)
--- 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 @@ found:
                        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: