[PATCH bpf v1 0/8] Misc bug fixes - part 3

[PATCH bpf v1 0/8] Misc bug fixes - part 3

[Kumar Kartikeya Dwivedi]

A set of miscellaneous fixes for bugs reported by Nicholas, batched
together again. See commit logs for details. Some of this was caught and
posted by Ning before, but AI raised some concerns, so I'm resolving
those issues and commandeering their patches now.

Kumar Kartikeya Dwivedi (4):
  bpf: Require MEM_PERCPU for percpu kptr stores
  selftests/bpf: Reject non-percpu values in percpu kptr fields
  bpf: Clear NON_OWN_REF after RCU protection ends
  selftests/bpf: Reject graph kptr use after RCU unlock

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

 include/linux/bpf_verifier.h                  |   4 +-
 kernel/bpf/verifier.c                         |  27 +++-
 .../selftests/bpf/progs/percpu_alloc_fail.c   |  59 +++++++
 .../selftests/bpf/progs/rcu_read_lock.c       |   6 +-
 .../selftests/bpf/progs/refcounted_kptr.c     |  61 ++++++++
 .../bpf/progs/refcounted_kptr_fail.c          | 148 ++++++++++++++++++
 6 files changed, 296 insertions(+), 9 deletions(-)


base-commit: 254c881fe0554c5efb16d355c273702a27a32a20
-- 
2.53.0

[PATCH bpf v1 1/8] bpf: Require MEM_PERCPU for percpu kptr stores

[Kumar Kartikeya Dwivedi]

map_kptr_match_type() treats perm_flags as the set of register type flags
that a kptr field permits. Adding MEM_PERCPU to that set for
BPF_KPTR_PERCPU does not require the source register to carry it, however.
The subset test consequently accepts both a plain bpf_obj_new() allocation
and a referenced kernel pointer into a __percpu_kptr map field.

Loads from the field are always marked MEM_PERCPU. Consumers then treat the
stored value as the cookie returned by bpf_percpu_obj_new(): per-CPU pointer
helpers relocate it, and map teardown selects the per-CPU free path. A plain
allocation can therefore provide an arbitrary kernel read/write, while a
kernel pointer can be relocated into an invalid address or sent through a
missing destructor.

Require the source MEM_PERCPU flag to match the destination field kind.
This preserves valid bpf_percpu_obj_new() stores and rejects both the
program-BTF and kernel-BTF variants.

Fixes: 36d8bdf75a93 ("bpf: Add alloc/xchg/direct_access support for local percpu kptr")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d7dd0befbd10..68353aa61fa1 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4488,6 +4488,13 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
 	if (type_flag(reg->type) & ~perm_flags)
 		goto bad_type;
 
+	/*
+	 * A BPF_KPTR_PERCPU field is read back as MEM_PERCPU, so the value
+	 * stored in it must carry the same flag.
+	 */
+	if ((kptr_field->type == BPF_KPTR_PERCPU) != !!(reg->type & MEM_PERCPU))
+		goto bad_type;
+
 	/* We need to verify reg->type and reg->btf, before accessing reg->btf */
 	reg_name = btf_type_name(reg->btf, reg->btf_id);
 
-- 
2.53.0

[PATCH bpf v1 2/8] selftests/bpf: Reject non-percpu values in percpu kptr fields

[Kumar Kartikeya Dwivedi]

Add verifier coverage for the two ways a non-percpu pointer can be stored
in a __percpu_kptr field: a program-BTF local allocation returned by
bpf_obj_new(), and a referenced kernel-BTF task_struct pointer.

Without the verifier fix, both programs are unexpectedly accepted and the
negative tests fail. Requiring MEM_PERCPU makes both programs fail
verification with the expected invalid-kptr diagnostic.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/percpu_alloc_fail.c   | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
index 08379c3b6a03..3701f4ea58c7 100644
--- a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
+++ b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
@@ -33,6 +33,20 @@ struct {
 	__type(value, struct elem);
 } array SEC(".maps");
 
+struct kernel_percpu_elem {
+	struct task_struct __percpu_kptr *task;
+};
+
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__uint(max_entries, 1);
+	__type(key, int);
+	__type(value, struct kernel_percpu_elem);
+} kernel_percpu_array SEC(".maps");
+
+struct task_struct *bpf_task_from_pid(s32 pid) __ksym;
+void bpf_task_release(struct task_struct *p) __ksym;
+
 long ret;
 
 SEC("?fentry/bpf_fentry_test1")
@@ -137,6 +151,51 @@ int BPF_PROG(test_array_map_5)
 	return 0;
 }
 
+SEC("?syscall")
+__failure __msg("invalid kptr access, R2 type=trusted_ptr_ expected=ptr_task_struct")
+int reject_kernel_ptr_into_percpu_kptr(void *ctx)
+{
+	struct kernel_percpu_elem *e;
+	struct task_struct *p, *old;
+	int index = 0;
+
+	e = bpf_map_lookup_elem(&kernel_percpu_array, &index);
+	if (!e)
+		return 0;
+
+	p = bpf_task_from_pid(1);
+	if (!p)
+		return 0;
+
+	old = bpf_kptr_xchg(&e->task, p);
+	if (old)
+		bpf_task_release(old);
+	return 0;
+}
+
+SEC("?fentry.s/bpf_fentry_test1")
+__failure __msg("invalid kptr access, R2 type=ptr_ expected=ptr_val_t")
+int BPF_PROG(reject_plain_alloc_into_percpu_kptr)
+{
+	struct val_t __percpu_kptr *old;
+	struct val_t *p;
+	struct elem *e;
+	int index = 0;
+
+	e = bpf_map_lookup_elem(&array, &index);
+	if (!e)
+		return 0;
+
+	p = bpf_obj_new(struct val_t);
+	if (!p)
+		return 0;
+
+	old = bpf_kptr_xchg(&e->pc, p);
+	if (old)
+		bpf_percpu_obj_drop(old);
+	return 0;
+}
+
 SEC("?fentry.s/bpf_fentry_test1")
 __failure __msg("bpf_percpu_obj_new type ID argument must be of a struct of scalars")
 int BPF_PROG(test_array_map_6)
-- 
2.53.0

[PATCH bpf v1 3/8] bpf: Keep refcount_acquire nullable for borrowed RCU kptrs

[Kumar Kartikeya Dwivedi]

From: Ning Ding <dingning04@gmail.com>

bpf_refcount_acquire() is fallible for a borrowed reference because the
object may have reached a zero refcount. The verifier therefore keeps
KF_RET_NULL on the return value unless the argument is an owning reference.

An RCU-protected load of a local kptr is marked MEM_ALLOC, but it only
receives NON_OWN_REF when the pointee contains a graph node. A refcounted
object without a graph node consequently looks like an owning reference
even though the loaded register has no acquired reference state. If the
program drops the last real reference while remaining in the RCU critical
section, refcount_inc_not_zero() returns NULL while the verifier treats the
result as non-NULL.

Only classify the argument as owning when it is backed by a verifier-tracked
reference. This retains the non-NULL return for pointers from bpf_obj_new(),
bpf_kptr_xchg(), or an earlier successful acquisition, while requiring a
NULL check for borrowed RCU kptrs.

Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Ning Ding <dingning04@gmail.com>
[ kkd: Rewrote commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@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 68353aa61fa1..bc0abf96cc89 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13178,7 +13178,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me
 						      bpf_diag_reg_type_plain(env, reg->type));
 				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.53.0

[PATCH bpf v1 4/8] selftests/bpf: Test borrowed refcount acquisition nullability

[Kumar Kartikeya Dwivedi]

From: Ning Ding <dingning04@gmail.com>

Add verifier coverage for the distinction between owning and borrowed
arguments to bpf_refcount_acquire().

An owning pointer returned by bpf_obj_new() must continue producing a
non-NULL result without an extra check. An RCU-loaded local kptr is only
borrowed, so a checked result must load successfully while passing an
unchecked result to bpf_obj_drop() must be rejected as possibly NULL.

Use a sleepable syscall program for the borrowed cases so the explicit RCU
critical section is what permits the local kptr load. Without the verifier
fix, the unchecked case is incorrectly accepted. With it, the verifier
rejects the possibly NULL argument.

Signed-off-by: Ning Ding <dingning04@gmail.com>
[ kkd: Rewrote commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/refcounted_kptr.c     | 61 +++++++++++++++++++
 .../bpf/progs/refcounted_kptr_fail.c          | 48 +++++++++++++++
 2 files changed, 109 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
index 61906f48025c..cae00f7b0a24 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("?syscall")
+__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 eaaed0859f94..7d2f8897e5ad 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;
@@ -89,6 +105,38 @@ long refcount_acquire_non_object(void *ctx)
 	return bpf_refcount_acquire(ctx) != NULL;
 }
 
+SEC("?syscall")
+__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;
+
+	/* Force Clang to emit complete BTF for struct node_refcount_only. */
+	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.53.0

[PATCH bpf v1 5/8] bpf: Clear NON_OWN_REF after RCU protection ends

[Kumar Kartikeya Dwivedi]

A local kptr load of an object containing a graph node is marked MEM_RCU
and NON_OWN_REF while protected by RCU. When the last RCU read-side critical
section ends, invalidate_rcu_protected_refs() removes MEM_RCU and marks the
pointer PTR_UNTRUSTED, but leaves NON_OWN_REF set.

The stale flag lets graph kfunc argument checks continue treating the
pointer as a live borrowed reference. In particular, bpf_rbtree_remove()
can accept a pointer after its protection ended and return it as a new
owning reference, even though the object may already have been freed.

Clear NON_OWN_REF when an RCU-protected pointer is demoted. A spin lock also
provides implicit RCU protection, so invalidate non-owning references before
demoting RCU-protected pointers when releasing the lock. Otherwise the
demotion would clear the flag before invalidate_non_owning_refs() can find
and invalidate those aliases.

The demoted pointer remains available for fault-protected reads. Exempt such
reads from the allocated-object reference-state assertion; writes through a
fault-prone pointer are already rejected, and bpf_may_fault_on_deref() makes
the surviving loads use BPF_PROBE_MEM.

Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bc0abf96cc89..56a10f79f9a8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6038,7 +6038,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) &&
+		/*
+		 * A fault-prone allocated object may still be read through a
+		 * BPF_PROBE_MEM load after its lifetime protection ends. Writes
+		 * through such pointers were rejected above.
+		 */
+		if (type_is_alloc(reg->type) && !bpf_may_fault_on_deref(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;
@@ -7416,10 +7422,14 @@ static int process_spin_lock(struct bpf_verifier_env *env, struct bpf_reg_state
 				lock);
 			return -EINVAL;
 		}
+		/*
+		 * Invalidate non-owning refs before RCU demotion clears their
+		 * NON_OWN_REF flag.
+		 */
+		invalidate_non_owning_refs(env);
+
 		if (!in_rcu_cs(env))
 			invalidate_rcu_protected_refs(env);
-
-		invalidate_non_owning_refs(env);
 	}
 	return 0;
 }
@@ -9519,7 +9529,7 @@ static void invalidate_rcu_protected_refs(struct bpf_verifier_env *env)
 	bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, stack, clear_mask, ({
 		if (reg->type & MEM_RCU) {
 			bpf_diag_mod_begin(env, reg, NULL, BPF_DIAG_MOD_WRITE);
-			reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
+			reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL | NON_OWN_REF);
 			reg->type |= PTR_UNTRUSTED;
 			bpf_diag_mod_end(env);
 		}
-- 
2.53.0

[PATCH bpf v1 6/8] selftests/bpf: Reject graph kptr use after RCU unlock

[Kumar Kartikeya Dwivedi]

Add a sleepable verifier test that loads a graph-node local kptr in an
explicit RCU read-side critical section, then passes its node to
bpf_rbtree_remove() after the section ends.

Before the verifier fix, the stale NON_OWN_REF flag makes the node look like
a live borrowed reference and the program is accepted. After the fix, the
pointer is demoted without NON_OWN_REF and the graph kfunc argument is
rejected.

Also exercise a graph kptr loaded while a spin lock provides implicit RCU
protection. The pointer must be invalidated when the lock is released, which
guards the required ordering between non-owning-reference invalidation and
RCU demotion.

Update the existing fault-protected load test state description. The
post-unlock pointer no longer carries NON_OWN_REF, but remains readable
because the load is rewritten to use BPF_PROBE_MEM.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/rcu_read_lock.c       |  6 +-
 .../bpf/progs/refcounted_kptr_fail.c          | 73 +++++++++++++++++++
 2 files changed, 76 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
index 31d4081c3a9f..cdb255addbc3 100644
--- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c
+++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
@@ -592,9 +592,9 @@ int non_own_ref_untrusted_ld(void *ctx)
 	}
 	bpf_rcu_read_unlock();
 	/*
-	 * The unlock leaves node as PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED
-	 * | NON_OWN_REF, and the load below has to get the BPF_PROBE_MEM
-	 * rewrite for it, otherwise a bad address panics the kernel.
+	 * The unlock leaves node as PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED,
+	 * and the load below has to get the BPF_PROBE_MEM rewrite for it,
+	 * otherwise a bad address panics the kernel.
 	 */
 	non_own_ref_key = node->key;
 	return 0;
diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
index 7d2f8897e5ad..5abd8387d26f 100644
--- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
@@ -28,6 +28,15 @@ struct map_value_refcount_only {
 	struct node_refcount_only __kptr *node;
 };
 
+struct rcu_graph_node {
+	struct bpf_rb_node node;
+	long data;
+};
+
+struct map_value_rcu_graph {
+	struct rcu_graph_node __kptr *node;
+};
+
 extern void bpf_rcu_read_lock(void) __ksym;
 extern void bpf_rcu_read_unlock(void) __ksym;
 
@@ -36,6 +45,8 @@ private(A) struct bpf_spin_lock glock;
 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);
+private(C) struct bpf_spin_lock graph_lock;
+private(C) struct bpf_rb_root graph_root __contains(rcu_graph_node, node);
 
 struct {
 	__uint(type, BPF_MAP_TYPE_ARRAY);
@@ -44,6 +55,13 @@ struct {
 	__uint(max_entries, 1);
 } stashed_refcount_only SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_ARRAY);
+	__type(key, int);
+	__type(value, struct map_value_rcu_graph);
+	__uint(max_entries, 1);
+} stashed_rcu_graph SEC(".maps");
+
 static bool less(struct bpf_rb_node *a, const struct bpf_rb_node *b)
 {
 	struct node_acquire *node_a;
@@ -137,6 +155,61 @@ long refcount_acquire_rcu_map_kptr_unchecked_drop(void *ctx)
 	return 0;
 }
 
+SEC("?syscall")
+__failure
+__msg("bpf_rbtree_remove can only take non-owning or refcounted "
+      "bpf_rb_node pointer")
+long rbtree_remove_after_rcu_unlock(void *ctx)
+{
+	struct map_value_rcu_graph *mapval;
+	struct bpf_rb_node *rb_node;
+	struct rcu_graph_node *node;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_rcu_graph, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_rcu_read_lock();
+	node = mapval->node;
+	if (!node) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+
+	bpf_spin_lock(&graph_lock);
+	rb_node = bpf_rbtree_remove(&graph_root, &node->node);
+	bpf_spin_unlock(&graph_lock);
+	if (rb_node)
+		bpf_obj_drop(container_of(rb_node, struct rcu_graph_node, node));
+
+	return 0;
+}
+
+SEC("?syscall")
+__failure __msg("invalid mem access 'scalar'")
+long graph_kptr_after_spin_unlock(void *ctx)
+{
+	struct map_value_rcu_graph *mapval;
+	struct rcu_graph_node *node;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_rcu_graph, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_spin_lock(&graph_lock);
+	node = mapval->node;
+	if (!node) {
+		bpf_spin_unlock(&graph_lock);
+		return 0;
+	}
+	bpf_spin_unlock(&graph_lock);
+
+	return node->data;
+}
+
 SEC("?tc")
 __failure __msg("Unreleased reference id=3 alloc_insn={{[0-9]+}}")
 long rbtree_refcounted_node_ref_escapes_owning_input(void *ctx)
-- 
2.53.0

[PATCH bpf v1 7/8] bpf: Reject untrusted allocated-object pointers

[Kumar Kartikeya Dwivedi]

From: Ning Ding <dingning04@gmail.com>

When the final RCU read-side critical section ends, a local kptr is demoted
to PTR_UNTRUSTED but retains MEM_ALLOC. The pointer may be NULL or may refer
to an object whose lifetime is no longer protected.

type_is_ptr_alloc_obj() nevertheless recognizes any PTR_TO_BTF_ID with
MEM_ALLOC as a live allocated object. In particular, a refcount-only local
kptr never carries NON_OWN_REF, so it still passes the
bpf_refcount_acquire() argument check after RCU protection ends. The kfunc
can then dereference NULL or stale memory.

Make type_is_ptr_alloc_obj() reject PTR_UNTRUSTED pointers. Since
type_is_non_owning_ref() is based on the same predicate, graph kfunc
arguments obey the same live-object requirement. Fault-protected reads of
the demoted pointer remain valid: writes are already rejected, and read
fixups use bpf_may_fault_on_deref() rather than this predicate.

Fixes: 1b12171533a9 ("bpf: Mark direct ld of stashed bpf_{rb,list}_node as non-owning ref")
Reported-by: Nicholas Carlini <npc@anthropic.com>
Suggested-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Ning Ding <dingning04@gmail.com>
[ kkd: Rewrote commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpf_verifier.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 1339c2f028db..36b65797877d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1381,7 +1381,9 @@ 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_non_owning_ref(u32 type)
-- 
2.53.0

[PATCH bpf v1 8/8] selftests/bpf: Reject refcount acquisition after RCU unlock

[Kumar Kartikeya Dwivedi]

From: Ning Ding <dingning04@gmail.com>

Add a sleepable verifier test that loads a refcount-only local kptr in an
explicit RCU read-side critical section, ends the section, and passes the
pointer to bpf_refcount_acquire().

The loaded pointer never carries NON_OWN_REF. After RCU unlock it retains
MEM_ALLOC while becoming PTR_UNTRUSTED, which previously made the kfunc
argument check accept it as a live allocated object. Expect verification to
reject the untrusted argument instead.

Signed-off-by: Ning Ding <dingning04@gmail.com>
[ kkd: Rewrote commit log ]
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../bpf/progs/refcounted_kptr_fail.c          | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
index 5abd8387d26f..7be45100c4d6 100644
--- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
@@ -187,6 +187,33 @@ long rbtree_remove_after_rcu_unlock(void *ctx)
 	return 0;
 }
 
+SEC("?syscall")
+__failure __msg("R1 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 *node, *ref;
+	int idx = 0;
+
+	mapval = bpf_map_lookup_elem(&stashed_refcount_only, &idx);
+	if (!mapval)
+		return 0;
+
+	bpf_rcu_read_lock();
+	node = mapval->node;
+	if (!node) {
+		bpf_rcu_read_unlock();
+		return 0;
+	}
+	bpf_rcu_read_unlock();
+
+	ref = bpf_refcount_acquire(node);
+	if (ref)
+		bpf_obj_drop(ref);
+
+	return 0;
+}
+
 SEC("?syscall")
 __failure __msg("invalid mem access 'scalar'")
 long graph_kptr_after_spin_unlock(void *ctx)
-- 
2.53.0

Re: [PATCH bpf v1 2/8] selftests/bpf: Reject non-percpu values in percpu kptr fields

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Newly introduced selftests assert on incomplete verifier error messages, cementing a preexisting verifier logging bug and making the tests brittle.
--

commit ecbf9b0933ebffdc896739df59e08ba01779b11c
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Reject non-percpu values in percpu kptr fields

This commit adds verifier coverage for the two ways a non-percpu pointer can
be stored in a __percpu_kptr field. It ensures that passing a program-BTF
local allocation or a referenced kernel-BTF task_struct pointer correctly
fails verification when MEM_PERCPU is required.

> diff --git a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
> index 08379c3b6a038..3701f4ea58c75 100644
> --- a/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
> +++ b/tools/testing/selftests/bpf/progs/percpu_alloc_fail.c
[ ... ]
> @@ -137,6 +151,51 @@ int BPF_PROG(test_array_map_5)
>  	return 0;
>  }
>  
> +SEC("?syscall")
> +__failure __msg("invalid kptr access, R2 type=trusted_ptr_ expected=ptr_task_struct")
> +int reject_kernel_ptr_into_percpu_kptr(void *ctx)

[Severity: Low]
Does this test hardcode an incomplete verifier error message resulting from an
uninitialized reg_name in the verifier?

In map_kptr_match_type(), when triggering the MEM_PERCPU mismatch, the
verifier jumps to bad_type before reg_name is populated. This leaves the
struct name out of the output (e.g., outputting "type=trusted_ptr_" instead
of "type=trusted_ptr_task_struct").

If the verifier logging bug is fixed to properly include the struct name,
this substring match will break. Could the test be made robust against
verifier log improvements, or could the verifier fix be included?

> +{
> +	struct kernel_percpu_elem *e;
[ ... ]
> +	old = bpf_kptr_xchg(&e->task, p);
> +	if (old)
> +		bpf_task_release(old);
> +	return 0;
> +}
> +
> +SEC("?fentry.s/bpf_fentry_test1")
> +__failure __msg("invalid kptr access, R2 type=ptr_ expected=ptr_val_t")
> +int BPF_PROG(reject_plain_alloc_into_percpu_kptr)

[Severity: Low]
Similarly, does this assertion also rely on the incomplete verifier string
(expecting "type=ptr_" instead of "type=ptr_val_t")?

> +{
> +	struct val_t __percpu_kptr *old;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904063650.3877826-1-memxor@gmail.com?part=2

Re: [PATCH bpf v1 6/8] selftests/bpf: Reject graph kptr use after RCU unlock

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/rcu_read_lock.c b/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> index 31d4081c3a9fd..cdb255addbc30 100644
> --- a/tools/testing/selftests/bpf/progs/rcu_read_lock.c
> +++ b/tools/testing/selftests/bpf/progs/rcu_read_lock.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> index 7d2f8897e5ad1..5abd8387d26f3 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> @@ -28,6 +28,15 @@ struct map_value_refcount_only {
>  	struct node_refcount_only __kptr *node;
>  };
>
> +struct rcu_graph_node {
> +	struct bpf_rb_node node;
> +	long data;
> +};
> +
> +struct map_value_rcu_graph {
> +	struct rcu_graph_node __kptr *node;
> +};
> +
>  extern void bpf_rcu_read_lock(void) __ksym;
>  extern void bpf_rcu_read_unlock(void) __ksym;
>
> @@ -36,6 +45,8 @@ private(A) struct bpf_spin_lock glock;
>  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);
> +private(C) struct bpf_spin_lock graph_lock;
> +private(C) struct bpf_rb_root graph_root __contains(rcu_graph_node, node);

Does struct rcu_graph_node have a forcing construct to ensure Clang emits
a complete BTF_KIND_STRUCT rather than just a forward declaration?

The struct is referenced only through pointer types in this file:

  struct rcu_graph_node __kptr *node;         (map value member)
  struct rcu_graph_node *node;                (pointer locals)
  container_of(rb_node, struct rcu_graph_node, node)

There is no bpf_obj_new(typeof(*n)), no sizeof(), and no by-value use.

The __contains() tag at the private(C) graph_root declaration requires
resolving rcu_graph_node by name as a struct. The kernel resolves this
strictly by name and kind in btf_find_graph_root():

    id = btf_find_by_name_kind(btf, value_type, BTF_KIND_STRUCT);

If Clang emits only a BTF_KIND_FWD for rcu_graph_node, this returns
-ENOENT, btf_parse_fields() fails for the .data.C internal ARRAY map, and
libbpf's bpf_object__create_maps() fails. That takes down the whole
refcounted_kptr_fail object, so every test in the file reports a failure
with a map-creation error rather than the asserted verifier message.

The immediately preceding commit in this series (1cf6db4cf60b4,
"selftests/bpf: Test borrowed refcount acquisition nullability") had to
add an explicit workaround in this same file for an identical shape:

    /* Force Clang to emit complete BTF for struct node_refcount_only. */
    tmp = bpf_obj_new(typeof(*tmp));
    if (!tmp)
        return 3;
    bpf_obj_drop(tmp);

rcu_graph_node has no equivalent construct. Surveying every __contains()
target in tools/testing/selftests/bpf/progs (node_acquire, node_refcounted,
node_data, skb_node, fq_flow_node, tree_node, tree_node_ref, foo, bar),
every one is instantiated with bpf_obj_new(typeof(*x)) somewhere in the
same object. rcu_graph_node is the first __contains() target in the tree
that never is, so it is the first to depend on pointer-only references
producing a complete struct.

Should this add a forcing construct for rcu_graph_node in the same style
as the sibling test, either a bpf_obj_new(typeof(*node))/bpf_obj_drop()
pair or a statement in the changelog that the emitted BTF was verified to
contain a full STRUCT rather than a FWD for rcu_graph_node?


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

Re: [PATCH bpf v1 4/8] selftests/bpf: Test borrowed refcount acquisition nullability

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c b/tools/testing/selftests/bpf/progs/refcounted_kptr_fail.c
> index eaaed0859f946..7d2f8897e5ad1 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;

[ ... ]

> @@ -89,6 +105,38 @@ long refcount_acquire_non_object(void *ctx)
>  	return bpf_refcount_acquire(ctx) != NULL;
>  }
>
> +SEC("?syscall")
> +__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;
> +
> +	/* Force Clang to emit complete BTF for struct node_refcount_only. */
> +	tmp = bpf_obj_new(typeof(*tmp));
> +	if (!tmp)
> +		return 3;
> +	bpf_obj_drop(tmp);

This isn't a bug, but would a file-scope declaration work here instead of
the bpf_obj_new()/bpf_obj_drop() prologue?

Looking at how other BPF selftests handle the BTF forward-reference issue,
tools/testing/selftests/bpf/progs/local_kptr_stash.c uses a simple
file-scope declaration:

    struct node_data *just_here_because_btf_bug;

and tools/testing/selftests/bpf/progs/rcu_read_lock.c does the same:

    struct rcu_node_data *just_here_because_btf_bug;

Both patterns force complete BTF emission without adding locals or return
paths to the test program.

Since this is a __failure test focused on verifier rejection of a possibly
NULL pointer, would the declarative approach keep the test more focused on
the pattern it is testing?

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


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