Re: [PATCH bpf v3 2/4] selftests/bpf: Test refcount_acquire return nullability
Amery Hung <[email protected]> Mon, 3 Aug 2026 06:59:49 -0700
| Newsgroups | org.kernel.vger.linux-kselftest,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <CAMB2axOMcg8ZKPR8GOmeOuKUyDKZ9WVvAgJvRU5kuTm+_tOcvA@mail.gmail.com> |
On Mon, Aug 3, 2026 at 4:26=E2=80=AFAM Ning Ding <[email protected]> wro= te: > > The verifier could accept an unchecked bpf_refcount_acquire() result for = a > borrowed RCU-loaded map kptr. If the call returns NULL, passing the resul= t > to bpf_obj_drop() can crash the kernel. > > Add tests showing that an owned input remains non-NULL, a checked borrowe= d > result is accepted, and an unchecked borrowed result is rejected. > > Assisted-by: Codex:gpt-5.5 > Assisted-by: ChatGPT:GPT-5.6-Thinking > Signed-off-by: Ning Ding <[email protected]> > --- > .../selftests/bpf/progs/refcounted_kptr.c | 61 +++++++++++++++++++ > .../bpf/progs/refcounted_kptr_fail.c | 47 ++++++++++++++ > 2 files changed, 108 insertions(+) > > diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/= testing/selftests/bpf/progs/refcounted_kptr.c > index 61906f48025cc..fd35093285c0d 100644 > --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c > +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c > @@ -23,6 +23,15 @@ struct map_value { > struct node_data __kptr *node; > }; > > +struct node_refcount_only { > + long key; > + struct bpf_refcount refcount; > +}; > + > +struct map_value_refcount_only { > + struct node_refcount_only __kptr *node; > +}; > + > struct { > __uint(type, BPF_MAP_TYPE_ARRAY); > __type(key, int); > @@ -30,6 +39,13 @@ struct { > __uint(max_entries, 2); > } stashed_nodes SEC(".maps"); > > +struct { > + __uint(type, BPF_MAP_TYPE_ARRAY); > + __type(key, int); > + __type(value, struct map_value_refcount_only); > + __uint(max_entries, 1); > +} stashed_refcount_only SEC(".maps"); > + > struct node_acquire { > long key; > long data; > @@ -832,6 +848,51 @@ long rbtree_refcounted_node_ref_escapes_owning_input= (void *ctx) > return 0; > } > > +SEC("tc") > +__success > +long refcount_acquire_owning_input_no_null_check(void *ctx) > +{ > + struct node_refcount_only *n, *m; > + > + n =3D bpf_obj_new(typeof(*n)); > + if (!n) > + return 1; > + > + m =3D bpf_refcount_acquire(n); > + bpf_obj_drop(m); > + bpf_obj_drop(n); > + > + return 0; > +} > + > +SEC("tc") > +__success > +long refcount_acquire_rcu_map_kptr_null_checked(void *ctx) > +{ > + struct map_value_refcount_only *mapval; > + struct node_refcount_only *n, *m; > + int idx =3D 0; > + > + mapval =3D bpf_map_lookup_elem(&stashed_refcount_only, &idx); > + if (!mapval) > + return 1; > + > + bpf_rcu_read_lock(); > + n =3D mapval->node; > + if (!n) { > + bpf_rcu_read_unlock(); > + return 2; > + } > + m =3D bpf_refcount_acquire(n); > + bpf_rcu_read_unlock(); > + > + if (!m) > + return 3; > + bpf_obj_drop(m); > + > + return 0; > +} > + > static long __stash_map_empty_xchg(struct node_data *n, int idx) > { > struct map_value *mapval =3D bpf_map_lookup_elem(&stashed_nodes, = &idx); > diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/t= ools/testing/selftests/bpf/progs/refcounted_kptr_fail.c > index 024ef2aae2008..acd3e81a39168 100644 > --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c > +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c > @@ -19,6 +19,15 @@ struct node_refcounted { > struct bpf_refcount refcount; > }; > > +struct node_refcount_only { > + long key; > + struct bpf_refcount refcount; > +}; > + > +struct map_value_refcount_only { > + struct node_refcount_only __kptr *node; > +}; > + > extern void bpf_rcu_read_lock(void) __ksym; > extern void bpf_rcu_read_unlock(void) __ksym; > > @@ -28,6 +37,13 @@ private(A) struct bpf_rb_root groot __contains(node_ac= quire, node); > private(B) struct bpf_spin_lock lock; > private(B) struct bpf_list_head head __contains(node_refcounted, list); > > +struct { > + __uint(type, BPF_MAP_TYPE_ARRAY); > + __type(key, int); > + __type(value, struct map_value_refcount_only); > + __uint(max_entries, 1); > +} stashed_refcount_only SEC(".maps"); > + > static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b) > { > struct node_acquire *node_a; > @@ -80,6 +96,37 @@ long refcount_acquire_maybe_null(void *ctx) > return 0; > } > > +SEC("?tc") > +__failure __msg("Possibly NULL pointer passed to trusted R1") > +long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx) > +{ > + struct map_value_refcount_only *mapval; > + struct node_refcount_only *tmp, *n, *m; > + int idx =3D 0; > + > + tmp =3D bpf_obj_new(typeof(*tmp)); > + if (!tmp) > + return 3; > + bpf_obj_drop(tmp); Could you explain the purpose of this chunk? Otherwise, it looks good to me. Reviewed-by: Amery Hung <[email protected]> > + > + mapval =3D bpf_map_lookup_elem(&stashed_refcount_only, &idx); > + if (!mapval) > + return 1; > + > + bpf_rcu_read_lock(); > + n =3D mapval->node; > + if (!n) { > + bpf_rcu_read_unlock(); > + return 2; > + } > + m =3D bpf_refcount_acquire(n); > + bpf_rcu_read_unlock(); > + > + bpf_obj_drop(m); > + > + return 0; > +} > + > SEC("?tc") > __failure __msg("Unreleased reference id=3D3 alloc_insn=3D{{[0-9]+}}") > long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx) > -- > 2.43.0 > >