[PATCH bpf-next v4 1/4] bpf: Keep refcount_acquire nullable for borrowed RCU kptrs

[PATCH bpf-next v4 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")
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 6ac1afced20b..bc021c8c8fbf 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12482,7 +12482,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
 					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-next v4 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 61906f48025c..fd35093285c0 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 024ef2aae200..acd3e81a3916 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

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

[bot+bpf-ci]

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

> 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

[PATCH bpf-next v4 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
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 27b43fda9b17..22fcd01c3cce 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1341,7 +1341,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 bc021c8c8fbf..c3a11c98cd45 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5820,7 +5820,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

Re: [PATCH bpf-next v4 3/4] bpf: Reject untrusted allocated-object pointers

[bot+bpf-ci]

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

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 27b43fda9b178..22fcd01c3cceb 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -1341,7 +1341,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)

Does adding the PTR_UNTRUSTED check to type_is_ptr_alloc_obj() also
change type_is_non_owning_ref()?

Looking at the definition in include/linux/bpf_verifier.h:

    static inline bool type_is_non_owning_ref(u32 type)
    {
            return type_is_ptr_alloc_obj(type) && type_flag(type) & NON_OWN_REF;
    }

It calls type_is_ptr_alloc_obj(), so after this patch it will also
reject PTR_UNTRUSTED registers. The commit message describes tightening
bpf_refcount_acquire() but doesn't mention this transitive effect.

Can this affect invalidate_non_owning_refs() in sleepable programs?

Consider a sleepable program using bpf_spin_lock() around graph
operations (which is a supported pattern, as seen in
tools/testing/selftests/bpf/progs/refcounted_kptr.c tests like
rbtree_sleepable_rcu).

When the program calls bpf_spin_unlock(), process_spin_lock() runs:

    if (!in_rcu_cs(env))
            invalidate_rcu_protected_refs(env);
    invalidate_non_owning_refs(env);

For sleepable programs, in_rcu_cs() returns false, so
invalidate_rcu_protected_refs() runs first. That function does:

    reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
    reg->type |= PTR_UNTRUSTED;

A register holding a non-owning ref to a stashed graph node starts as
PTR_TO_BTF_ID|MEM_ALLOC|NON_OWN_REF|MEM_RCU (set by
btf_ld_kptr_type()/ref_set_non_owning() when loading from the graph).
After invalidate_rcu_protected_refs() it becomes
PTR_TO_BTF_ID|MEM_ALLOC|NON_OWN_REF|PTR_UNTRUSTED.

The immediately following invalidate_non_owning_refs() only acts on
registers where type_is_non_owning_ref() returns true. But after this
patch, type_is_non_owning_ref() returns false for registers with
PTR_UNTRUSTED.

So these registers that should be invalidated instead survive
bpf_spin_unlock() as live, readable pointers. Is this intended?

Pre-patch they would have been invalidated by mark_reg_invalid() in
invalidate_non_owning_refs(). Post-patch they escape invalidation.

The commit message states untrusted pointers "are still allowed for
BPF_PROBE_MEM reads," but does the BPF_PROBE_MEM conversion actually
work for the NON_OWN_REF variant?

Looking at kernel/bpf/fixups.c, the probe conversion logic has:

    case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
            /* probe mem access */
            ...
    default:
            continue;

It has a case for the plain MEM_ALLOC|PTR_UNTRUSTED type, but no case
for PTR_TO_BTF_ID|MEM_ALLOC|NON_OWN_REF|PTR_UNTRUSTED. So a load
through the surviving register is emitted as a plain load with no
exception-table entry, which would fault in the kernel instead of
returning zero.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index bc021c8c8fbf7..c3a11c98cd451 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5820,7 +5820,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;

This change allows the demoted PTR_UNTRUSTED|NON_OWN_REF registers to
reach mark_btf_ld_reg() instead of hitting the sanity assertion, which
opens the path for the read-through described above.


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

Re: [PATCH bpf-next v4 3/4] bpf: Reject untrusted allocated-object pointers

[Ning Ding]

> The immediately following invalidate_non_owning_refs() only acts on
> registers where type_is_non_owning_ref() returns true. But after this
> patch, type_is_non_owning_ref() returns false for registers with
> PTR_UNTRUSTED.

Makes sense, I think the thing is, callers do not have a consistent
contract of `type_is_non_owning_ref()` (same for
`type_is_ptr_alloc_obj()`). Some of them only care about whether a
register has the NON_OWN_REF flag or not, but some callers assume it
also should not contain the PTR_UNTRUSTED flag. So if I make some
callers happy, then other callers probably will complain because I
break their assumption.

Then I think the previous approach, which preserves the old semantics:
`type_is_ptr_alloc_obj()` only checks the flags it declared in its
name and doesn't care about whether other flags are here or not will
work better. If some caller explicitly need to ensure the register
does not have PTR_UNTRUSTED, they should check it by themselves.

> The commit message states untrusted pointers "are still allowed for
> BPF_PROBE_MEM reads," but does the BPF_PROBE_MEM conversion actually
> work for the NON_OWN_REF variant?
>
> Looking at kernel/bpf/fixups.c, the probe conversion logic has:
>
>     case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
>             /* probe mem access */
>             ...
>     default:
>             continue;
>
> It has a case for the plain MEM_ALLOC|PTR_UNTRUSTED type, but no case
> for PTR_TO_BTF_ID|MEM_ALLOC|NON_OWN_REF|PTR_UNTRUSTED. So a load
> through the surviving register is emitted as a plain load with no
> exception-table entry, which would fault in the kernel instead of
> returning zero.
I didn't consider the NON_OWN_REF combination case, but I think that
should be a separate patch for fixups.c?

> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index bc021c8c8fbf7..c3a11c98cd451 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -5820,7 +5820,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;
>
> This change allows the demoted PTR_UNTRUSTED|NON_OWN_REF registers to
> reach mark_btf_ld_reg() instead of hitting the sanity assertion, which
> opens the path for the read-through described above.

I think for the exemption, that is intended, ths bad thing is about
the fixups.c does not handle
PTR_TO_BTF_ID|MEM_ALLOC|NON_OWN_REF|PTR_UNTRUSTED.

Overall, there are 2 separate problems:
1. type_is_non_owning_ref should be fixed to preserve the original semantics
2. For the exemption, new cases should be added to fixups.c.

For problem one, I think there are 2 approaches:
1. Preserve the old type_is_ptr_alloc_obj() code, but for any caller
who wants to verify that the register is also trusted, they should
check themselves explicitly.
2. Change the type_is_non_owning_ref:
static inline bool type_is_non_owning_ref(u32 type)
{
return (type_is_ptr_alloc_obj(type) ||
type_is_untrusted_ptr_alloc_obj(type)) &&
      type_flag(type) & NON_OWN_REF;
}

[PATCH bpf-next v4 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 fd35093285c0..383c5b1b7111 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("?syscall")
+__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("?syscall")
+__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("?syscall")
+__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("?syscall")
+__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 acd3e81a3916..0cc4cbd0c81b 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("?syscall")
+__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-next v4 4/4] selftests/bpf: Test untrusted allocated-object pointers

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> index fd35093285c0d..383c5b1b7111f 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("?syscall")
> +__success
> +long map_kptr_read_after_rcu_unlock(void *ctx)

A reviewer noted in v3 that the tests use TC programs with implicit RCU
protection, causing pointers not to become untrusted as intended, and
specifically suggested: "The right fix might be to use fentry.s for these
tests to trigger untrusted marking of pointers."

v4 uses SEC("?syscall") instead. While syscall programs are sleepable and
may trigger untrusted marking, does this section annotation achieve the
same behavior Kumar recommended fentry.s for? The deviation from the
specific suggestion wasn't discussed in the v4 thread.

> +{
> +	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("?syscall")
> +__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("?syscall")
> +__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;
> +}

Does this test actually verify the fix? The "only read is supported"
rejection comes from the PTR_UNTRUSTED check in check_ptr_to_btf_access():

    if (atype != BPF_READ && (type_flag(reg->type) & PTR_UNTRUSTED)) {
            verbose(env, "only read is supported\n");
            return -EACCES;
    }

This check is unchanged in the pre-series tree and rejects writes through
any PTR_UNTRUSTED register for both object layouts. A kernel missing the
series fix would still fail this test with the same message, so the test
wouldn't catch a revert.

> +
> +SEC("?syscall")
> +__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;
> +}

Is __success correct for this read? After bpf_rcu_read_unlock() in a
sleepable program, the pointer gains PTR_UNTRUSTED but keeps NON_OWN_REF
because the fix excludes PTR_UNTRUSTED from type_is_ptr_alloc_obj().

At the load, the type is PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED |
NON_OWN_REF. The bpf_convert_ctx_accesses() switch in
kernel/bpf/fixups.c lists these BPF_PROBE_MEM cases:

    case PTR_TO_BTF_ID | PTR_UNTRUSTED:
    case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:

but not the NON_OWN_REF variant, so the switch falls through and the load
stays a plain BPF_MEM access with no exception-table entry.

The object is freed via bpf_mem_free_rcu(), so in a sleepable syscall
program that has left the RCU critical section, a grace period can elapse
and the page can be returned to the allocator before the load executes,
causing an unprotected fault.

Pre-series, invalidate_non_owning_refs() at bpf_spin_unlock() killed this
register because type_is_non_owning_ref() was true. Now that
type_is_ptr_alloc_obj() excludes PTR_UNTRUSTED, type_is_non_owning_ref()
returns false and the stale register survives.

The sibling test map_kptr_read_after_rcu_unlock (node_refcount_only, no
graph node, hence no NON_OWN_REF) does match a BPF_PROBE_MEM case and gets
the probe rewrite. Should either fixups.c gain a NON_OWN_REF case and this
test document the probe path, or should NON_OWN_REF be stripped in
invalidate_rcu_protected_refs()?

> +
>  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..0cc4cbd0c81b1 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("?syscall")
> +__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)

The commit message states:

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

Is the description fully accurate?

The "direct write" was never allowed for any object layout. The
PTR_UNTRUSTED check in check_ptr_to_btf_access() predates the series and
rejects writes through any PTR_UNTRUSTED register for both node_data and
node_refcount_only.

The claim "keep BPF_PROBE_MEM reads accepted ... Cover both tested object
layouts" is only true for node_refcount_only (map_kptr_read_after_rcu_unlock).
The node_data read (graph_map_kptr_read_after_spin_unlock) ends up with
PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED | NON_OWN_REF, a combination
missing from bpf_convert_ctx_accesses() in kernel/bpf/fixups.c, so it
stays a plain BPF_MEM load without probe rewrite.

The genuinely new coverage is the two bpf_refcount_acquire() tests. Would
it be more accurate to scope the description to those plus the
refcount-only read?


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