bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref
authorDave Marchevsky <davemarchevsky@fb.com>
Tue, 7 Nov 2023 08:56:38 +0000 (00:56 -0800)
committerAlexei Starovoitov <ast@kernel.org>
Fri, 10 Nov 2023 03:07:51 +0000 (19:07 -0800)
This patch enables the following pattern:

  /* mapval contains a __kptr pointing to refcounted local kptr */
  mapval = bpf_map_lookup_elem(&map, &idx);
  if (!mapval || !mapval->some_kptr) { /* omitted */ }

  p = bpf_refcount_acquire(&mapval->some_kptr);

Currently this doesn't work because bpf_refcount_acquire expects an
owning or non-owning ref. The verifier defines non-owning ref as a type:

  PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF

while mapval->some_kptr is PTR_TO_BTF_ID | PTR_UNTRUSTED. It's possible
to do the refcount_acquire by first bpf_kptr_xchg'ing mapval->some_kptr
into a temp kptr, refcount_acquiring that, and xchg'ing back into
mapval, but this is unwieldy and shouldn't be necessary.

This patch modifies btf_ld_kptr_type such that user-allocated types are
marked MEM_ALLOC and if those types have a bpf_{rb,list}_node they're
marked NON_OWN_REF as well. Additionally, due to changes to
bpf_obj_drop_impl earlier in this series, rcu_protected_object now
returns true for all user-allocated types, resulting in
mapval->some_kptr being marked MEM_RCU.

After this patch's changes, mapval->some_kptr is now:

  PTR_TO_BTF_ID | MEM_ALLOC | NON_OWN_REF | MEM_RCU

which results in it passing the non-owning ref test, and the motivating
example passing verification.

Future work will likely get rid of special non-owning ref lifetime logic
in the verifier, at which point we'll be able to delete the NON_OWN_REF
flag entirely.

Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Link: https://lore.kernel.org/r/20231107085639.3016113-6-davemarchevsky@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
kernel/bpf/verifier.c

index 993e4677bbe992f55121b8b5450e411558788f56..9ae6eae1347164423583c200745c78d41d07d24a 100644 (file)
@@ -5557,10 +5557,23 @@ BTF_SET_END(rcu_protected_types)
 static bool rcu_protected_object(const struct btf *btf, u32 btf_id)
 {
        if (!btf_is_kernel(btf))
-               return false;
+               return true;
        return btf_id_set_contains(&rcu_protected_types, btf_id);
 }
 
+static struct btf_record *kptr_pointee_btf_record(struct btf_field *kptr_field)
+{
+       struct btf_struct_meta *meta;
+
+       if (btf_is_kernel(kptr_field->kptr.btf))
+               return NULL;
+
+       meta = btf_find_struct_meta(kptr_field->kptr.btf,
+                                   kptr_field->kptr.btf_id);
+
+       return meta ? meta->record : NULL;
+}
+
 static bool rcu_safe_kptr(const struct btf_field *field)
 {
        const struct btf_field_kptr *kptr = &field->kptr;
@@ -5571,12 +5584,25 @@ static bool rcu_safe_kptr(const struct btf_field *field)
 
 static u32 btf_ld_kptr_type(struct bpf_verifier_env *env, struct btf_field *kptr_field)
 {
+       struct btf_record *rec;
+       u32 ret;
+
+       ret = PTR_MAYBE_NULL;
        if (rcu_safe_kptr(kptr_field) && in_rcu_cs(env)) {
-               if (kptr_field->type != BPF_KPTR_PERCPU)
-                       return PTR_MAYBE_NULL | MEM_RCU;
-               return PTR_MAYBE_NULL | MEM_RCU | MEM_PERCPU;
+               ret |= MEM_RCU;
+               if (kptr_field->type == BPF_KPTR_PERCPU)
+                       ret |= MEM_PERCPU;
+               else if (!btf_is_kernel(kptr_field->kptr.btf))
+                       ret |= MEM_ALLOC;
+
+               rec = kptr_pointee_btf_record(kptr_field);
+               if (rec && btf_record_has_field(rec, BPF_GRAPH_NODE))
+                       ret |= NON_OWN_REF;
+       } else {
+               ret |= PTR_UNTRUSTED;
        }
-       return PTR_MAYBE_NULL | PTR_UNTRUSTED;
+
+       return ret;
 }
 
 static int check_map_kptr_access(struct bpf_verifier_env *env, u32 regno,