bpf: Fix check_reg_type for PTR_TO_BTF_ID
authorKumar Kartikeya Dwivedi <memxor@gmail.com>
Tue, 21 Feb 2023 20:06:44 +0000 (21:06 +0100)
committerAlexei Starovoitov <ast@kernel.org>
Wed, 22 Feb 2023 20:50:15 +0000 (12:50 -0800)
The current code does type matching for the case where reg->type is
PTR_TO_BTF_ID or has the PTR_TRUSTED flag. However, this only needs to
occur for non-MEM_ALLOC and non-MEM_PERCPU cases, but will include both
as per the current code.

The MEM_ALLOC case with or without PTR_TRUSTED needs to be handled
specially by the code for type_is_alloc case, while MEM_PERCPU case must
be ignored. Hence, to restore correct behavior and for clarity,
explicitly list out the handled PTR_TO_BTF_ID types which should be
handled for each case using a switch statement.

Helpers currently only take:
PTR_TO_BTF_ID
PTR_TO_BTF_ID | PTR_TRUSTED
PTR_TO_BTF_ID | MEM_RCU
PTR_TO_BTF_ID | MEM_ALLOC
PTR_TO_BTF_ID | MEM_PERCPU
PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED

This fix was also described (for the MEM_ALLOC case) in [0].

  [0]: https://lore.kernel.org/bpf/20221121160657.h6z7xuvedybp5y7s@apollo

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20230221200646.2500777-6-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
kernel/bpf/verifier.c

index 477c22c..062fd4a 100644 (file)
@@ -6527,7 +6527,14 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
        return -EACCES;
 
 found:
-       if (reg->type == PTR_TO_BTF_ID || reg->type & PTR_TRUSTED) {
+       if (base_type(reg->type) != PTR_TO_BTF_ID)
+               return 0;
+
+       switch ((int)reg->type) {
+       case PTR_TO_BTF_ID:
+       case PTR_TO_BTF_ID | PTR_TRUSTED:
+       case PTR_TO_BTF_ID | MEM_RCU:
+       {
                /* For bpf_sk_release, it needs to match against first member
                 * 'struct sock_common', hence make an exception for it. This
                 * allows bpf_sk_release to work for multiple socket types.
@@ -6563,13 +6570,23 @@ found:
                                return -EACCES;
                        }
                }
-       } else if (type_is_alloc(reg->type)) {
+               break;
+       }
+       case PTR_TO_BTF_ID | MEM_ALLOC:
                if (meta->func_id != BPF_FUNC_spin_lock && meta->func_id != BPF_FUNC_spin_unlock) {
                        verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");
                        return -EFAULT;
                }
+               /* Handled by helper specific checks */
+               break;
+       case PTR_TO_BTF_ID | MEM_PERCPU:
+       case PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED:
+               /* Handled by helper specific checks */
+               break;
+       default:
+               verbose(env, "verifier internal error: invalid PTR_TO_BTF_ID register for type match\n");
+               return -EFAULT;
        }
-
        return 0;
 }