Re: [PATCH bpf v3 2/4] selftests/bpf: Test refcount_acquire return nullability

[Amery Hung]

On Mon, Aug 3, 2026 at 4:26 AM Ning Ding <dingning04@xxxxxxxxx> wrote:
>
> The verifier could accept an unchecked bpf_refcount_acquire() result for a
> borrowed RCU-loaded map kptr. If the call returns NULL, passing the result
> to bpf_obj_drop() can crash the kernel.
>
> Add tests showing that an owned input remains non-NULL, a checked borrowed
> 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 <dingning04@xxxxxxxxx>
> ---
>  .../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 = bpf_obj_new(typeof(*n));
> +       if (!n)
> +               return 1;
> +
> +       m = 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 = 0;
> +
> +       mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
> +       if (!mapval)
> +               return 1;
> +
> +       bpf_rcu_read_lock();
> +       n = mapval->node;
> +       if (!n) {
> +               bpf_rcu_read_unlock();
> +               return 2;
> +       }
> +       m = 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 = bpf_map_lookup_elem(&stashed_nodes, &idx);
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/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_acquire, 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 = 0;
> +
> +       tmp = 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 <ameryhung@xxxxxxxxx>

> +
> +       mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
> +       if (!mapval)
> +               return 1;
> +
> +       bpf_rcu_read_lock();
> +       n = mapval->node;
> +       if (!n) {
> +               bpf_rcu_read_unlock();
> +               return 2;
> +       }
> +       m = bpf_refcount_acquire(n);
> +       bpf_rcu_read_unlock();
> +
> +       bpf_obj_drop(m);
> +
> +       return 0;
> +}
> +
>  SEC("?tc")
>  __failure __msg("Unreleased reference id=3 alloc_insn={{[0-9]+}}")
>  long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx)
> --
> 2.43.0
>
>