Re: [PATCH bpf-next v4 2/4] selftests/bpf: Test refcount_acquire return nullability

> 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

[ ... ]

> @@ -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();

This isn't a bug, but since SEC("tc") programs are already treated as
being in an RCU critical section (in_rcu_cs() returns true for
non-sleepable programs), would a sleepable program type make the
bpf_rcu_read_lock()/unlock() section here actually load-bearing, or
could the pair simply be dropped?

> +
> +	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

[ ... ]

> @@ -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);

This isn't a bug, but is the bpf_obj_new()/bpf_obj_drop() pair at the
start needed to force a complete BTF type for struct node_refcount_only,
or is it leftover setup? A one-line comment would keep someone from
removing it later.

> +
> +	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)


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31745514567