[PATCH bpf v3 0/4] bpf: Fix refcount_acquire handling for borrowed kptrs

[PATCH bpf v3 0/4] bpf: Fix refcount_acquire handling for borrowed kptrs

[Ning Ding]

Fix two verifier bugs involving bpf_refcount_acquire() and map kptrs.

First, the verifier can mistake an RCU-loaded map kptr for an owned
reference and assume bpf_refcount_acquire() cannot return NULL. The kfunc
can return NULL after the last reference is dropped, so passing an
unchecked result to bpf_obj_drop() can crash the kernel.

Second, type_is_ptr_alloc_obj() accepts PTR_UNTRUSTED pointers. This lets a
pointer that is no longer protected by RCU reach bpf_refcount_acquire(). If
the object was freed and its address reused, the verifier may acquire a
reference through stale memory.

Patches 1 and 2 fix and test the return-value nullability. Patches 3 and 4
reject untrusted allocated-object pointers while keeping safe
BPF_PROBE_MEM reads accepted.

The series is based on bpf commit e5fd3f514e27
("bpf: tcp: Fix use-after-free in bpf_iter_tcp_established_batch()").

Testing used fresh virtme-ng KVM boots with KASAN enabled:

  pre-fix graph write:       accepted, as expected
  post-fix focused cases:    8/8 passed
  full refcount groups:      47/47 passed
  related list/rbtree groups: 167/167 passed
  skipped or failed cases:   none
  KASAN/Oops/panic markers:  none

---
v3:
  - Make type_is_ptr_alloc_obj() reject PTR_UNTRUSTED, as suggested by
    Kumar Kartikeya Dwivedi.
  - Add a separate helper for the BPF_PROBE_MEM read path.
  - Add read, refcount-acquire, and direct-write verifier tests.
  - Add Cc: stable@vger.kernel.org to both fixes, as requested by Greg
    Kroah-Hartman.
  - Split each fix from its selftests.

v2: https://lore.kernel.org/r/20260726235030.1152542-1-dingning04@gmail.com
v1: https://lore.kernel.org/r/20260726015330.705259-1-dingning04@gmail.com

Ning Ding (4):
  bpf: Keep refcount_acquire nullable for borrowed RCU kptrs
  selftests/bpf: Test refcount_acquire return nullability
  bpf: Reject untrusted allocated-object pointers
  selftests/bpf: Test untrusted allocated-object pointers

 include/linux/bpf_verifier.h                  |  11 +-
 kernel/bpf/verifier.c                         |  10 +-
 .../selftests/bpf/progs/refcounted_kptr.c     | 161 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_fail.c          |  74 ++++++++
 4 files changed, 253 insertions(+), 3 deletions(-)


base-commit: e5fd3f514e27db1f05fbd72ba615d74941e23c51
-- 
2.43.0

[PATCH bpf v3 1/4] bpf: Keep refcount_acquire nullable for borrowed RCU kptrs

[Ning Ding]

The verifier can mistake an RCU-loaded map kptr for an owned reference and
assume bpf_refcount_acquire() cannot return NULL. The kfunc uses
refcount_inc_not_zero(), so it can return NULL after the last reference is
dropped. Passing that unchecked result to bpf_obj_drop() can crash the
kernel.

Only treat the argument as owned when the verifier tracks an acquired
reference for it. Borrowed pointers remain nullable, while owned pointers
keep the existing behavior.

Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5.5
Assisted-by: ChatGPT:GPT-5.6-Thinking
Signed-off-by: Ning Ding <dingning04@gmail.com>
---
 kernel/bpf/verifier.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fdc5fbb1f78ca..c47328be2505e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12419,7 +12419,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
 					reg_arg_name(env, argno));
 				return -EINVAL;
 			}
-			if (!type_is_non_owning_ref(reg->type))
+			if (!type_is_non_owning_ref(reg->type) && reg_is_referenced(env, reg))
 				meta->arg_owning_ref = true;
 
 			rec = reg_btf_record(reg);
-- 
2.43.0

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

[Ning Ding]

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@gmail.com>
---
 .../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);
+
+	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

[PATCH bpf v3 3/4] bpf: Reject untrusted allocated-object pointers

[Ning Ding]

type_is_ptr_alloc_obj() currently treats PTR_UNTRUSTED pointers as valid
allocated objects. This allows a pointer that is no longer protected by
RCU to be passed to bpf_refcount_acquire(). If the object was freed and
its address reused, the verifier may acquire a reference through stale
memory.

Make type_is_ptr_alloc_obj() reject PTR_UNTRUSTED. Add
type_is_untrusted_ptr_alloc_obj() for the read path, where these pointers
are still allowed for BPF_PROBE_MEM reads. This keeps checks strict
without breaking safe reads.

Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")
Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/r/20260726021304.97ED91F000E9@smtp.kernel.org
Cc: stable@vger.kernel.org
Assisted-by: Codex:gpt-5
Signed-off-by: Ning Ding <dingning04@gmail.com>
---
 include/linux/bpf_verifier.h | 11 ++++++++++-
 kernel/bpf/verifier.c        |  8 +++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 39a851e690ec4..7014d62fe3d1c 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1307,7 +1307,16 @@ static inline bool bpf_type_has_unsafe_modifiers(u32 type)
 
 static inline bool type_is_ptr_alloc_obj(u32 type)
 {
-	return base_type(type) == PTR_TO_BTF_ID && type_flag(type) & MEM_ALLOC;
+	return base_type(type) == PTR_TO_BTF_ID &&
+	       type_flag(type) & MEM_ALLOC &&
+	       !(type_flag(type) & PTR_UNTRUSTED);
+}
+
+static inline bool type_is_untrusted_ptr_alloc_obj(u32 type)
+{
+	return base_type(type) == PTR_TO_BTF_ID &&
+	       type_flag(type) & MEM_ALLOC &&
+	       type_flag(type) & PTR_UNTRUSTED;
 }
 
 static inline bool type_is_non_owning_ref(u32 type)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c47328be2505e..8b35c63780798 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5809,7 +5809,13 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
 			return -EACCES;
 		}
 
-		if (type_is_alloc(reg->type) && !type_is_non_owning_ref(reg->type) &&
+		/*
+		 * Skip this referenced-ID sanity check for untrusted allocated objects;
+		 * the access check above already rejects writes through them.
+		 */
+		if (type_is_alloc(reg->type) &&
+		    !type_is_untrusted_ptr_alloc_obj(reg->type) &&
+		    !type_is_non_owning_ref(reg->type) &&
 		    !(reg->type & MEM_RCU) && !reg_is_referenced(env, reg)) {
 			verifier_bug(env, "allocated object must have a referenced id");
 			return -EFAULT;
-- 
2.43.0

[PATCH bpf v3 4/4] selftests/bpf: Test untrusted allocated-object pointers

[Ning Ding]

The verifier previously allowed pointers used after RCU protection ended
to reach bpf_refcount_acquire() and, for one object layout, a direct write.
If the object was freed and reused, these operations could access stale
memory.

Add tests that keep BPF_PROBE_MEM reads accepted but reject reference
acquisition and direct writes after RCU protection ends. Cover both tested
object layouts.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/r/20260726021304.97ED91F000E9@smtp.kernel.org
Assisted-by: Codex:gpt-5
Signed-off-by: Ning Ding <dingning04@gmail.com>
---
 .../selftests/bpf/progs/refcounted_kptr.c     | 100 ++++++++++++++++++
 .../bpf/progs/refcounted_kptr_fail.c          |  27 +++++
 2 files changed, 127 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
index fd35093285c0d..b70be8b52ff80 100644
--- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
@@ -893,6 +893,106 @@ long refcount_acquire_rcu_map_kptr_null_checked(void *ctx)
 	return 0;
 }
 
+SEC("?tc")
+__success
+long map_kptr_read_after_rcu_unlock(void *ctx)
+{
+	struct map_value_refcount_only *mapval;
+	struct node_refcount_only *n;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_rcu_read_lock();
+	n = mapval->node;
+	if (!n) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+
+	return n->key;
+}
+
+SEC("?tc")
+__failure __msg("is neither owning or non-owning ref")
+long refcount_acquire_graph_after_rcu_unlock(void *ctx)
+{
+	struct map_value *mapval;
+	struct node_data *n, *m;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_rcu_read_lock();
+	n = mapval->node;
+	if (!n) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+
+	m = bpf_refcount_acquire(n);
+	if (m)
+		bpf_obj_drop(m);
+
+	return 0;
+}
+
+SEC("?tc")
+__failure __msg("only read is supported")
+long graph_map_kptr_write_after_rcu_unlock(void *ctx)
+{
+	struct map_value *mapval;
+	struct node_data *n;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
+	if (!mapval)
+		return 1;
+
+	bpf_rcu_read_lock();
+	n = mapval->node;
+	if (!n) {
+		bpf_rcu_read_unlock();
+		return 2;
+	}
+	bpf_rcu_read_unlock();
+
+	n->key = 1;
+	return 0;
+}
+
+SEC("?tc")
+__success
+long graph_map_kptr_read_after_spin_unlock(void *ctx)
+{
+	struct map_value *mapval;
+	struct node_data *n;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_nodes, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_rcu_read_lock();
+	n = mapval->node;
+	if (!n) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+
+	bpf_spin_lock(&lock);
+	bpf_spin_unlock(&lock);
+
+	return n->key;
+}
+
 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 acd3e81a39168..3408f68ad444d 100644
--- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
@@ -127,6 +127,33 @@ long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx)
 	return 0;
 }
 
+SEC("?tc")
+__failure __msg("is neither owning or non-owning ref")
+long refcount_acquire_after_rcu_unlock(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;
+	}
+	bpf_rcu_read_unlock();
+
+	m = bpf_refcount_acquire(n);
+	if (m)
+		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

Re: [PATCH bpf v3 1/4] bpf: Keep refcount_acquire nullable for borrowed RCU kptrs

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]

> bpf: Keep refcount_acquire nullable for borrowed RCU kptrs
>
> The verifier can mistake an RCU-loaded map kptr for an owned reference and
> assume bpf_refcount_acquire() cannot return NULL. The kfunc uses
> refcount_inc_not_zero(), so it can return NULL after the last reference is
> dropped. Passing that unchecked result to bpf_obj_drop() can crash the
> kernel.
>
> Only treat the argument as owned when the verifier tracks an acquired
> reference for it. Borrowed pointers remain nullable, while owned pointers
> keep the existing behavior.
>
> Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")

Does this Fixes tag point to the right commit?

Looking at the code change, the fix adds && reg_is_referenced(env, reg)
to an existing condition that checks !type_is_non_owning_ref(reg->type).
The problem being fixed is that the verifier incorrectly assumes pointers
that are not non-owning refs must be owning refs, which fails to account
for borrowed RCU pointers.

The commit 1b12171533a9 added logic to mark certain pointers as
non-owning refs, but the bug appears to originate from commit
7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for non-owning
refs"), which introduced the logic:

  if (!type_is_non_owning_ref(reg->type))
      meta->arg_owning_ref = true;

That commit incorrectly assumed pointers that are not non-owning refs
must be owning refs, missing the case of borrowed RCU pointers which are
neither. The current fix addresses this by verifying the verifier tracks
an acquired reference before treating the pointer as owned.

Should the Fixes tag be:

  Fixes: 7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for non-owning refs")


---
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/30810949269

Re: [PATCH bpf v3 1/4] bpf: Keep refcount_acquire nullable for borrowed RCU kptrs

[Amery Hung]

On Mon, Aug 3, 2026 at 6:14 AM <bot+bpf-ci@kernel.org> wrote:
>
> > bpf: Keep refcount_acquire nullable for borrowed RCU kptrs
> >
> > The verifier can mistake an RCU-loaded map kptr for an owned reference and
> > assume bpf_refcount_acquire() cannot return NULL. The kfunc uses
> > refcount_inc_not_zero(), so it can return NULL after the last reference is
> > dropped. Passing that unchecked result to bpf_obj_drop() can crash the
> > kernel.
> >
> > Only treat the argument as owned when the verifier tracks an acquired
> > reference for it. Borrowed pointers remain nullable, while owned pointers
> > keep the existing behavior.
> >
> > Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")

^^^ The fix tag is correct.

Reviewed-by: Amery Hung <ameryhung@gmail.com>

>
> Does this Fixes tag point to the right commit?
>
> Looking at the code change, the fix adds && reg_is_referenced(env, reg)
> to an existing condition that checks !type_is_non_owning_ref(reg->type).
> The problem being fixed is that the verifier incorrectly assumes pointers
> that are not non-owning refs must be owning refs, which fails to account
> for borrowed RCU pointers.
>
> The commit 1b12171533a9 added logic to mark certain pointers as
> non-owning refs, but the bug appears to originate from commit
> 7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for non-owning
> refs"), which introduced the logic:
>
>   if (!type_is_non_owning_ref(reg->type))
>       meta->arg_owning_ref = true;
>
> That commit incorrectly assumed pointers that are not non-owning refs
> must be owning refs, missing the case of borrowed RCU pointers which are
> neither. The current fix addresses this by verifying the verifier tracks
> an acquired reference before treating the pointer as owned.
>
> Should the Fixes tag be:
>
>   Fixes: 7793fc3babe9 ("bpf: Make bpf_refcount_acquire fallible for non-owning refs")
>
>
> ---
> 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/30810949269

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@gmail.com> 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@gmail.com>
> ---
>  .../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@gmail.com>

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

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

[Ning Ding]

> > +       tmp = bpf_obj_new(typeof(*tmp));
> > +       if (!tmp)
> > +               return 3;
> > +       bpf_obj_drop(tmp);

This dummy code creates a temporary node_refcount_only object and
immediately releases it, the actual bug test starts afterward. Its
purpose is compiler scaffolding for BTF: The dummy bpf_obj_new()
forces Clang to emit the complete BTF definition of that type,
including its bpf_refcount field. bpf_obj_drop() then releases the
temporary object so it won't affect the actual bug checking code.

If we remove the dummy block, Clang might not include the complete
node_refcount_only description in BTF. The program could then fail
during loading (e.g. while creating the map, since it can't validate
the kptr's complete type, or during verification, since the verifier
is not sure the object contains bpf_refcount field) before reaching
the actual test. (whether it still works depends on the clang version)