[PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[Ihor Solodrai]

A callback-calling helper or kfunc can pass its callback a pointer
that is only valid for the duration of the call, and the verifier has
no way to express that.

Two helpers need fixing:

  * bpf_user_ringbuf_drain() passes a CONST_PTR_TO_DYNPTR over a
    sample it releases as soon as the callback returns. When parked in
    callback_ctx, that register describes reused kernel stack and
    gives the program an arbitrary kernel read and write (bpf-next
    only, see patch #2).

  * bpf_for_each_map_elem() over an (percpu-) array map passes the
    address of a u32 held in bpf_for_each_array_elem()'s own frame,
    leaking four bytes of kernel stack.

Neither has a local fix: both arguments point into a helper's own
frame, with nothing longer-lived to anchor them to.

Introduce REF_TYPE_FRAME for this use case: a reference owned by a
callee frame, dropped when the frame is popped. Then use this
mechanism in both bpf_user_ringbuf_drain() and bpf_for_each_map_elem()
callee state setup.

---

The series is composed as follows:
  * patch #1 implements REF_TYPE_FRAME and relevant infra code, but
    it's not used yet
  * patches #2 and #5 use the new mark_frame_scoped_arg() helper
  * patch #3 adds relevant diagnostics
  * patches #4 and #6 add selftests to cover the changes

---

Ihor Solodrai (6):
  bpf: Introduce REF_TYPE_FRAME in the verifier
  bpf: Scope the bpf_user_ringbuf_drain() dynptr to its callback frame
  bpf: Name the callback in frame-release diagnostics
  selftests/bpf: Cover the user ringbuf callback dynptr lifetime
  bpf: Scope the bpf_for_each_map_elem() array key to the callback frame
  selftests/bpf: Cover callback-frame map key lifetime

 include/linux/bpf.h                           |   1 +
 include/linux/bpf_verifier.h                  |  11 +-
 kernel/bpf/arraymap.c                         |  18 ++-
 kernel/bpf/diagnostics.c                      |   3 +
 kernel/bpf/diagnostics.h                      |   1 +
 kernel/bpf/states.c                           |   4 +
 kernel/bpf/verifier.c                         | 149 +++++++++++++++---
 .../selftests/bpf/prog_tests/cb_refs.c        |   4 +-
 .../selftests/bpf/progs/exceptions_fail.c     |  20 +++
 .../selftests/bpf/progs/user_ringbuf_fail.c   | 143 +++++++++++++++++
 .../bpf/progs/verifier_iterating_callbacks.c  |  91 +++++++++++
 11 files changed, 419 insertions(+), 26 deletions(-)


base-commit: 79dc258c9392051420a26f1504c647bd3d27c66a
-- 
2.55.0

[PATCH bpf-next v1 1/6] bpf: Introduce REF_TYPE_FRAME in the verifier

[Ihor Solodrai]

A callback-calling helper can pass its callback a pointer that is only
valid for the duration of the call. The verifier just gives such an
argument a register type, which allows the callback to park the value,
or something derived from it, and make it outlive the frame. The BPF
program then can reuse it after the helper returns.

Invalidating a value together with everything derived from it is what
release_reference() already does, walking reg->parent_id across every
frame and stack slot. What is missing is a type of reference that is
not an object the program acquired and releases.

Introduce REF_TYPE_FRAME: a reference owned by a callee frame.
A set_callee_state_fn can declare an argument frame-scoped, and
setup_func_entry() turns the declaration into a reference, and
prepare_func_exit() drops it when the frame is popped, invalidating
the argument and everything derived from it through the existing walk.

Keep the new type invisible to find_reference_state(). release_reg()
and ref_convert_owning_non_owning() look up purely by id and could
otherwise destroy the anchor.

Fix up a few pre-existing comments while at it.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

---

This patch only introduces the mechanism: no helper declares a
frame-scoped argument yet. It is used in the subsequent patches in the
series, each fixing a separate bug.

---
---
 include/linux/bpf.h          |   1 +
 include/linux/bpf_verifier.h |  11 +++-
 kernel/bpf/states.c          |   4 ++
 kernel/bpf/verifier.c        | 121 ++++++++++++++++++++++++++++++++---
 4 files changed, 125 insertions(+), 12 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index fd22db8bc6c5..d849e4873417 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3118,6 +3118,7 @@ int bpf_iter_map_fill_link_info(const struct bpf_iter_aux_info *aux,
 int map_set_for_each_callback_args(struct bpf_verifier_env *env,
 				   struct bpf_func_state *caller,
 				   struct bpf_func_state *callee);
+void mark_frame_scoped_arg(struct bpf_func_state *callee, u32 regno);
 
 int bpf_percpu_hash_copy(struct bpf_map *map, void *key, void *value, u64 flags);
 int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value, u64 flags);
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 92f528c45605..51c310ea4e82 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -307,15 +307,13 @@ struct bpf_stack_state {
 };
 
 struct bpf_reference_state {
-	/* Each reference object has a type. Ensure REF_TYPE_PTR is zero to
-	 * default to pointer reference on zero initialization of a state.
-	 */
 	enum ref_state_type {
 		REF_TYPE_PTR		= (1 << 1),
 		REF_TYPE_IRQ		= (1 << 2),
 		REF_TYPE_LOCK		= (1 << 3),
 		REF_TYPE_RES_LOCK 	= (1 << 4),
 		REF_TYPE_RES_LOCK_IRQ	= (1 << 5),
+		REF_TYPE_FRAME		= (1 << 6),
 		REF_TYPE_LOCK_MASK	= REF_TYPE_LOCK | REF_TYPE_RES_LOCK | REF_TYPE_RES_LOCK_IRQ,
 	} type;
 	/* Track each reference created with a unique id, even if the same
@@ -333,6 +331,8 @@ struct bpf_reference_state {
 		 * it matches on unlock.
 		 */
 		void *ptr;
+		/* For REF_TYPE_FRAME */
+		u32 frameno;
 	};
 };
 
@@ -387,6 +387,11 @@ struct bpf_func_state {
 	u32 callback_depth;
 	/* Instructions processed in this frame and callees on the current path. */
 	u32 insns_subtotal;
+	/*
+	 * Set for arguments valid until the frame is popped.
+	 * Consumed by setup_func_entry().
+	 */
+	u16 frame_scoped_args;
 
 	/* The following fields should be last. See copy_func_state() */
 	/* The state of the stack. Each element of the array describes BPF_REG_SIZE
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c6a7..bf7efe5bcdca 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -898,6 +898,10 @@ static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *c
 			break;
 		case REF_TYPE_IRQ:
 			break;
+		case REF_TYPE_FRAME:
+			if (old->refs[i].frameno != cur->refs[i].frameno)
+				return false;
+			break;
 		case REF_TYPE_LOCK:
 		case REF_TYPE_RES_LOCK:
 		case REF_TYPE_RES_LOCK_IRQ:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d62c0f74cff5..a0a9d3d18f63 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1474,14 +1474,13 @@ static int grow_stack_arg_slots(struct bpf_verifier_env *env,
 	return 0;
 }
 
-/* Acquire a pointer id from the env and update the state->refs to include
- * this new pointer reference.
- * On success, returns a valid pointer id to associate with the register
- * On failure, returns a negative errno.
+/* Append an entry to @state->refs and record the instruction that created it.
+ * The caller fills in the type and the id.
+ * On success, returns the new entry. On failure, returns NULL.
  */
-static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
+static struct bpf_reference_state *__acquire_reference_state(struct bpf_verifier_state *state,
+							     int insn_idx)
 {
-	struct bpf_verifier_state *state = env->cur_state;
 	int new_ofs = state->acquired_refs;
 	int err;
 
@@ -1493,6 +1492,12 @@ static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_e
 	return &state->refs[new_ofs];
 }
 
+static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env,
+							   int insn_idx)
+{
+	return __acquire_reference_state(env->cur_state, insn_idx);
+}
+
 static int acquire_reference(struct bpf_verifier_env *env, int insn_idx, int parent_id)
 {
 	struct bpf_reference_state *s;
@@ -1507,6 +1512,31 @@ static int acquire_reference(struct bpf_verifier_env *env, int insn_idx, int par
 	return s->id;
 }
 
+/* Acquire a reference owned by frame @frameno of @state */
+static int acquire_frame_reference(struct bpf_verifier_env *env, struct bpf_verifier_state *state,
+				   int insn_idx, u32 frameno)
+{
+	struct bpf_reference_state *s;
+
+	s = __acquire_reference_state(state, insn_idx);
+	if (!s)
+		return -ENOMEM;
+	s->type = REF_TYPE_FRAME;
+	s->id = ++env->id_gen;
+	s->frameno = frameno;
+	return s->id;
+}
+
+/*
+ * Declare that @regno in @callee holds a value that stops being valid once the
+ * frame is popped. setup_func_entry() turns each declaration into a frame-owned
+ * reference.
+ */
+void mark_frame_scoped_arg(struct bpf_func_state *callee, u32 regno)
+{
+	callee->frame_scoped_args |= BIT(regno);
+}
+
 static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum ref_state_type type,
 			      int id, void *ptr)
 {
@@ -10175,7 +10205,7 @@ static int release_reference(struct bpf_verifier_env *env, int id)
 				continue;
 
 			/* Free objects derived from the current object */
-			if (reg->parent_id == id) {
+			if (reg->parent_id == id && reg->id != id) {
 				err = idstack_push(idstack, reg->id);
 				if (err)
 					return err;
@@ -10206,6 +10236,34 @@ static int release_reference(struct bpf_verifier_env *env, int id)
 	return 0;
 }
 
+/* Find the first reference owned by frame @frameno, or 0 if it owns none. */
+static u32 frame_reference_id(struct bpf_verifier_state *state, u32 frameno)
+{
+	int i;
+
+	for (i = 0; i < state->acquired_refs; i++)
+		if (state->refs[i].type == REF_TYPE_FRAME &&
+		    state->refs[i].frameno == frameno)
+			return state->refs[i].id;
+
+	return 0;
+}
+
+static int release_frame_reference(struct bpf_verifier_env *env, int id)
+{
+	struct bpf_verifier_state *state = env->cur_state;
+	int i;
+
+	for (i = 0; i < state->acquired_refs; i++) {
+		if (state->refs[i].type != REF_TYPE_FRAME || state->refs[i].id != id)
+			continue;
+		release_reference_state(state, i);
+		break;
+	}
+
+	return release_reference(env, id);
+}
+
 static void invalidate_non_owning_refs(struct bpf_verifier_env *env)
 {
 	struct bpf_func_state *unused;
@@ -10301,7 +10359,8 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
 			    struct bpf_verifier_state *state)
 {
 	struct bpf_func_state *caller, *callee;
-	int err;
+	u16 scoped_args;
+	int err, regno;
 
 	if (state->curframe + 1 >= MAX_CALL_FRAMES) {
 		verbose(env, "the call stack of %d frames is too deep\n",
@@ -10333,6 +10392,30 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
 	if (err)
 		goto err_out;
 
+	scoped_args = callee->frame_scoped_args;
+	callee->frame_scoped_args = 0;
+	for (regno = 0; regno < MAX_BPF_REG; regno++) {
+		int id;
+
+		if (!(scoped_args & BIT(regno)))
+			continue;
+
+		id = acquire_frame_reference(env, state, callsite, callee->frameno);
+		if (id < 0) {
+			err = id;
+			goto err_out;
+		}
+		/*
+		 * The value is its own lifetime anchor: there is no associated
+		 * object to borrow from, only the frame. parent_id = id here
+		 * covers both possible derived references:
+		 *  - through the id (e.g. dynptr slice)
+		 *  - through parent_id (e.g. dynptr clone)
+		 */
+		callee->regs[regno].id = id;
+		callee->regs[regno].parent_id = id;
+	}
+
 	/* only increment it after check_reg_arg() finished */
 	state->curframe++;
 
@@ -10560,6 +10643,10 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
 		if (err)
 			return err;
 
+		if (verifier_bug_if(callee->frame_scoped_args, env,
+				    "frame-scoped argument declared for async callback"))
+			return -EFAULT;
+
 		return 0;
 	}
 
@@ -11033,7 +11120,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 	struct bpf_func_state *caller, *callee;
 	struct bpf_reg_state *r0;
 	bool in_callback_fn;
-	u32 i, nregs;
+	u32 i, nregs, id;
 	int err;
 
 	callee = state->frame[state->curframe];
@@ -11106,6 +11193,17 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 		verbose(env, "to caller at %d:\n", *insn_idx);
 		print_verifier_state(env, state, caller->frameno, true);
 	}
+
+	/*
+	 * Values the caller only guaranteed for the duration of the call stop
+	 * being valid here.
+	 */
+	while ((id = frame_reference_id(state, callee->frameno))) {
+		err = release_frame_reference(env, id);
+		if (err)
+			return err;
+	}
+
 	account_processed_insns(env, callee, caller);
 	/* clear everything in the callee. In case of exceptional exits using
 	 * bpf_throw, this will be done by copy_verifier_state for extra frames. */
@@ -11283,6 +11381,11 @@ static int check_reference_leak(struct bpf_verifier_env *env, bool exception_exi
 		return 0;
 
 	for (i = 0; i < state->acquired_refs; i++) {
+		if (!exception_exit && state->refs[i].type == REF_TYPE_FRAME) {
+			verifier_bug(env, "frame %u reference id=%d alive at program exit",
+				     state->refs[i].frameno, state->refs[i].id);
+			return -EFAULT;
+		}
 		if (state->refs[i].type != REF_TYPE_PTR)
 			continue;
 		/* Allow struct_ops programs to return a referenced kptr back to
-- 
2.55.0

[PATCH bpf-next v1 2/6] bpf: Scope the bpf_user_ringbuf_drain() dynptr to its callback frame

[Ihor Solodrai]

bpf_user_ringbuf_drain() peeks a sample, initialises a struct
bpf_dynptr_kern on its own stack, passes it to the callback, and calls
__bpf_user_ringbuf_sample_release() as soon as the callback returns.
Neither the descriptor nor the sample it describes is valid afterwards.

set_user_ringbuf_callback_state() only gives the callback's R1 a type.
The callback can therefore store the CONST_PTR_TO_DYNPTR register into
callback_ctx, which points into a frame that outlives the call, and the
program can use it after the drain returns. Three routes reach past the
callback:

  - the register itself, spillable since v7.3-rc1, which points at a
    descriptor on reused kernel stack and gives the program an arbitrary
    kernel read/write

  - a bpf_dynptr_data() or bpf_dynptr_slice() result, which carries
    the dynptr's id as its parent_id

  - a bpf_dynptr_clone(), which is a by-value copy in the caller's
    frame and can be sliced after the drain returns

Declare R1 frame-scoped so all three are invalidated when the callback
frame is popped.

Reported-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/verifier.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a0a9d3d18f63..9775a6d38d3b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -707,11 +707,15 @@ static void mark_dynptr_stack_regs(struct bpf_verifier_env *env,
 	__mark_dynptr_reg(sreg2, type, false, id, parent_id);
 }
 
-static void mark_dynptr_cb_reg(struct bpf_verifier_env *env,
-			       struct bpf_reg_state *reg,
+/*
+ * A callback dynptr argument is valid only until the frame is popped, so
+ * setup_func_entry() assigns its id along with the frame reference.
+ */
+static void mark_dynptr_cb_reg(struct bpf_func_state *callee, u32 regno,
 			       enum bpf_dynptr_type type)
 {
-	__mark_dynptr_reg(reg, type, true, ++env->id_gen, 0);
+	__mark_dynptr_reg(&callee->regs[regno], type, true, 0, 0);
+	mark_frame_scoped_arg(callee, regno);
 }
 
 static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
@@ -10962,7 +10966,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
 	 * callback_fn(const struct bpf_dynptr_t* dynptr, void *callback_ctx);
 	 */
 	bpf_mark_reg_not_init(env, &callee->regs[BPF_REG_0]);
-	mark_dynptr_cb_reg(env, &callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL);
+	mark_dynptr_cb_reg(callee, BPF_REG_1, BPF_DYNPTR_TYPE_LOCAL);
 	callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3];
 
 	/* unused */
-- 
2.55.0

[PATCH bpf-next v1 3/6] bpf: Name the callback in frame-release diagnostics

[Ihor Solodrai]

A frame-owned reference is dropped through the same descendant walk as a
program-owned one, so a program that uses a callback argument after the
callback returns is told "resource release invalidated this value".
No resource was released, and nothing the program did caused it.

Add __release_reference() with the reason parameter.

The message names the callback rather than the frame because a
callback is the only thing that declares a frame-scoped argument
today, and it is what the program author recognises.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/diagnostics.c |  3 +++
 kernel/bpf/diagnostics.h |  1 +
 kernel/bpf/verifier.c    | 18 +++++++++++-------
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
index 5ecfa86ed49f..0610c3f6b334 100644
--- a/kernel/bpf/diagnostics.c
+++ b/kernel/bpf/diagnostics.c
@@ -2233,6 +2233,9 @@ static void diag_print_mod(struct bpf_verifier_env *env, const struct bpf_diag_h
 								   "resource release invalidated "
 								   "this value";
 		break;
+	case BPF_DIAG_MOD_FRAME_RELEASE:
+		reason = "the callback that owned this value returned";
+		break;
 	case BPF_DIAG_MOD_PKT_DATA_CHANGE:
 		reason = "packet data may have moved";
 		break;
diff --git a/kernel/bpf/diagnostics.h b/kernel/bpf/diagnostics.h
index a4102fb049ec..b5cab4d79c1a 100644
--- a/kernel/bpf/diagnostics.h
+++ b/kernel/bpf/diagnostics.h
@@ -22,6 +22,7 @@ enum bpf_diag_mod_reason {
 	BPF_DIAG_MOD_SPILL,
 	BPF_DIAG_MOD_VAR_WRITE,
 	BPF_DIAG_MOD_REF_RELEASE,
+	BPF_DIAG_MOD_FRAME_RELEASE,
 	BPF_DIAG_MOD_PKT_DATA_CHANGE,
 	BPF_DIAG_MOD_NON_OWN_REF,
 	BPF_DIAG_MOD_CALLER_SAVED,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9775a6d38d3b..ba0c8c27b45f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10169,7 +10169,8 @@ static int idstack_pop(struct bpf_idmap *idmap)
 }
 
 /* Release id and objects derived from it iteratively in a DFS manner */
-static int release_reference(struct bpf_verifier_env *env, int id)
+static int __release_reference(struct bpf_verifier_env *env, int id,
+			       enum bpf_diag_mod_reason reason)
 {
 	u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR);
 	struct bpf_verifier_state *vstate = env->cur_state;
@@ -10224,14 +10225,12 @@ static int release_reference(struct bpf_verifier_env *env, int id)
 
 				if (reg->dynptr.first_slot)
 					dyn_stack--;
-				bpf_diag_record_scrub(env, &dyn_stack[0].spilled_ptr,
-						      BPF_DIAG_MOD_REF_RELEASE);
-				bpf_diag_record_scrub(env, &dyn_stack[1].spilled_ptr,
-						      BPF_DIAG_MOD_REF_RELEASE);
+				bpf_diag_record_scrub(env, &dyn_stack[0].spilled_ptr, reason);
+				bpf_diag_record_scrub(env, &dyn_stack[1].spilled_ptr, reason);
 				invalidate_dynptr(env, dyn_stack);
 				continue;
 			}
-			bpf_diag_record_scrub(env, reg, BPF_DIAG_MOD_REF_RELEASE);
+			bpf_diag_record_scrub(env, reg, reason);
 			if (!stack || stack->slot_type[BPF_REG_SIZE - 1] == STACK_SPILL)
 				mark_reg_invalid(env, reg);
 		}));
@@ -10240,6 +10239,11 @@ static int release_reference(struct bpf_verifier_env *env, int id)
 	return 0;
 }
 
+static int release_reference(struct bpf_verifier_env *env, int id)
+{
+	return __release_reference(env, id, BPF_DIAG_MOD_REF_RELEASE);
+}
+
 /* Find the first reference owned by frame @frameno, or 0 if it owns none. */
 static u32 frame_reference_id(struct bpf_verifier_state *state, u32 frameno)
 {
@@ -10265,7 +10269,7 @@ static int release_frame_reference(struct bpf_verifier_env *env, int id)
 		break;
 	}
 
-	return release_reference(env, id);
+	return __release_reference(env, id, BPF_DIAG_MOD_FRAME_RELEASE);
 }
 
 static void invalidate_non_owning_refs(struct bpf_verifier_env *env)
-- 
2.55.0

[PATCH bpf-next v1 4/6] selftests/bpf: Cover the user ringbuf callback dynptr lifetime

[Ihor Solodrai]

Add a rejection test for each route the bpf_user_ringbuf_drain() callback
dynptr can take out of its frame:

  - the CONST_PTR_TO_DYNPTR register parked in callback_ctx
  - a bpf_dynptr_data() slice
  - a bpf_dynptr_slice() slice
  - a bpf_dynptr_clone() written into the caller's frame
  - a slice taken from that clone after the drain returns
  - an inner drain's dynptr escaping into an outer callback

The first also checks that the diagnostic names the callback.

bpf_throw() from the callback reaches check_reference_leak() with the
frame reference still live, and is rejected afterwards by
check_max_stack_depth(). It matches that rejection in full, because
"bpf_throw" alone also matches the reference-leak wording the callback
must not produce.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 .../selftests/bpf/progs/exceptions_fail.c     |  20 +++
 .../selftests/bpf/progs/user_ringbuf_fail.c   | 143 ++++++++++++++++++
 2 files changed, 163 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index 22503cf62e9f..86a0667ba348 100644
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -31,6 +31,11 @@ struct {
 	__type(value, struct hmap_elem);
 } hmap SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_USER_RINGBUF);
+	__uint(max_entries, 4096);
+} user_ringbuf SEC(".maps");
+
 private(A) struct bpf_spin_lock lock;
 private(A) struct bpf_rb_root rbtree __contains(foo, node);
 
@@ -110,6 +115,21 @@ static int timer_cb(void *map, int *key, struct bpf_timer *timer)
 	return 0;
 }
 
+static long drain_cb(struct bpf_dynptr *dynptr, void *context)
+{
+	bpf_throw(0);
+	return 0;
+}
+
+SEC("?tc")
+__failure
+__msg("bpf_throw kfunc (insn {{[0-9]+}}) cannot be called from callback subprog {{[0-9]+}}")
+int reject_user_ringbuf_callback_throw(struct __sk_buff *ctx)
+{
+	bpf_user_ringbuf_drain(&user_ringbuf, drain_cb, NULL, 0);
+	return 0;
+}
+
 SEC("?tc")
 __failure __msg("cannot be called from callback subprog")
 int reject_async_callback_throw(struct __sk_buff *ctx)
diff --git a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
index c0d0422b8030..a8d3acfa2bd2 100644
--- a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
+++ b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
@@ -1,9 +1,11 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
 
+#include <stdbool.h>
 #include <linux/bpf.h>
 #include <bpf/bpf_helpers.h>
 #include "bpf_misc.h"
+#include "bpf_kfuncs.h"
 
 char _license[] SEC("license") = "GPL";
 
@@ -243,3 +245,144 @@ int user_ringbuf_callback_const_ptr_to_dynptr_reg_off(void *ctx)
 			       callback_adjust_bpf_dynptr_reg_off, NULL, 0);
 	return 0;
 }
+
+/* The sample goes back to the producer as soon as the callback returns. */
+struct dynptr_ctx {
+	struct bpf_dynptr *saved;
+};
+
+static long callback_park_dynptr(struct bpf_dynptr *dynptr, void *context)
+{
+	struct dynptr_ctx *c = context;
+
+	c->saved = dynptr;
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("the callback that owned this value returned")
+int user_ringbuf_callback_park_dynptr(void *ctx)
+{
+	struct dynptr_ctx c = {};
+	char buf[8] = {};
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_dynptr, &c, 0);
+	if (c.saved)
+		bpf_dynptr_read(buf, sizeof(buf), c.saved, 0, 0);
+	return buf[0];
+}
+
+struct slice_ctx {
+	char *p;
+};
+
+static long callback_park_data_slice(struct bpf_dynptr *dynptr, void *context)
+{
+	struct slice_ctx *c = context;
+
+	c->p = bpf_dynptr_data(dynptr, 0, 8);
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("the callback that owned this value returned")
+int user_ringbuf_callback_park_data_slice(void *ctx)
+{
+	struct slice_ctx c = {};
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_data_slice, &c, 0);
+	if (c.p)
+		return c.p[0];
+	return 0;
+}
+
+static long callback_park_kfunc_slice(struct bpf_dynptr *dynptr, void *context)
+{
+	struct slice_ctx *c = context;
+
+	c->p = bpf_dynptr_slice(dynptr, 0, NULL, 8);
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("the callback that owned this value returned")
+int user_ringbuf_callback_park_kfunc_slice(void *ctx)
+{
+	struct slice_ctx c = {};
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_kfunc_slice, &c, 0);
+	if (c.p)
+		return c.p[0];
+	return 0;
+}
+
+struct clone_ctx {
+	struct bpf_dynptr clone;
+	__u64 armed;
+};
+
+static long callback_park_clone(struct bpf_dynptr *dynptr, void *context)
+{
+	struct clone_ctx *c = context;
+
+	bpf_dynptr_clone(dynptr, &c->clone);
+	c->armed = 1;
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("Expected an initialized dynptr as R3")
+int user_ringbuf_callback_park_clone(void *ctx)
+{
+	struct clone_ctx c = {};
+	char buf[8] = {};
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_clone, &c, 0);
+	if (c.armed)
+		bpf_dynptr_read(buf, sizeof(buf), &c.clone, 0, 0);
+	return buf[0];
+}
+
+SEC("?raw_tp")
+__failure __msg("Expected an initialized dynptr as R1")
+int user_ringbuf_callback_park_clone_then_slice(void *ctx)
+{
+	struct clone_ctx c = {};
+	char *p;
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_clone, &c, 0);
+	if (c.armed) {
+		p = bpf_dynptr_data(&c.clone, 0, 8);
+		if (p)
+			return p[0];
+	}
+	return 0;
+}
+
+static long callback_park_inner(struct bpf_dynptr *dynptr, void *context)
+{
+	struct dynptr_ctx *c = context;
+
+	c->saved = dynptr;
+	return 0;
+}
+
+/* An inner drain's dynptr must not escape into the outer callback either. */
+static long callback_park_outer(struct bpf_dynptr *dynptr, void *context)
+{
+	struct dynptr_ctx inner = {};
+	char buf[8] = {};
+
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_inner, &inner, 0);
+	if (inner.saved)
+		bpf_dynptr_read(buf, sizeof(buf), inner.saved, 0, 0);
+	return buf[0] ? 1 : 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("the callback that owned this value returned")
+int user_ringbuf_callback_nested_park_inner(void *ctx)
+{
+	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_outer, NULL, 0);
+	return 0;
+}
-- 
2.55.0

[PATCH bpf-next v1 5/6] bpf: Scope the bpf_for_each_map_elem() array key to the callback frame

[Ihor Solodrai]

bpf_for_each_array_elem() passes the callback the address of a u32 held
in its own stack frame:

	u32 i, key, num_elems = 0;
	...
		key = i;
		ret = callback_fn((u64)(long)map, (u64)(long)&key, ...);

The callback can store that PTR_TO_MAP_KEY into callback_ctx and the
program can dereference it after the iteration finishes. Loads through
PTR_TO_MAP_KEY are not fault-protected and array key_size is fixed at 4,
so it is a four-byte read-only leak of kernel stack.

Declare the key frame-scoped for the array map ops rather than in the
shared map_set_for_each_callback_args(): of the map_for_each_callback
implementations, only array and percpu-array pass a key from their own
frame. The hash family passes elem->key, which stays valid for as long
as the program runs.

map_key_from_value() does the same for array maps, for the timer, wq and
task_work callbacks. Those are left alone: their signature is
(map, key, value) with R4 and R5 uninitialised, so there is no
callback_ctx to park a typed pointer in, and a pointer written into map
memory loses its type.

The element value is unaffected in every case: it lives until map
teardown.

The frame-owned reference consumes an id, so leak_prog and nested_cb now
report id 5 rather than id 4; update the expected messages.

Reported-by: Nicholas Carlini <npc@anthropic.com>
Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 kernel/bpf/arraymap.c                          | 18 ++++++++++++++++--
 .../testing/selftests/bpf/prog_tests/cb_refs.c |  4 ++--
 2 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 0ce26b538075..44bd229873ca 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -855,6 +855,20 @@ static u64 array_map_mem_usage(const struct bpf_map *map)
 	return usage;
 }
 
+static int array_map_set_for_each_callback_args(struct bpf_verifier_env *env,
+						struct bpf_func_state *caller,
+						struct bpf_func_state *callee)
+{
+	int err;
+
+	err = map_set_for_each_callback_args(env, caller, callee);
+	if (err)
+		return err;
+
+	mark_frame_scoped_arg(callee, BPF_REG_2);
+	return 0;
+}
+
 BTF_ID_LIST_SINGLE(array_map_btf_ids, struct, bpf_array)
 const struct bpf_map_ops array_map_ops = {
 	.map_meta_equal = array_map_meta_equal,
@@ -875,7 +889,7 @@ const struct bpf_map_ops array_map_ops = {
 	.map_check_btf = array_map_check_btf,
 	.map_lookup_batch = generic_map_lookup_batch,
 	.map_update_batch = generic_map_update_batch,
-	.map_set_for_each_callback_args = map_set_for_each_callback_args,
+	.map_set_for_each_callback_args = array_map_set_for_each_callback_args,
 	.map_for_each_callback = bpf_for_each_array_elem,
 	.map_mem_usage = array_map_mem_usage,
 	.map_btf_id = &array_map_btf_ids[0],
@@ -900,7 +914,7 @@ const struct bpf_map_ops percpu_array_map_ops = {
 	.map_check_btf = array_map_check_btf,
 	.map_lookup_batch = generic_map_lookup_batch,
 	.map_update_batch = generic_map_update_batch,
-	.map_set_for_each_callback_args = map_set_for_each_callback_args,
+	.map_set_for_each_callback_args = array_map_set_for_each_callback_args,
 	.map_for_each_callback = bpf_for_each_array_elem,
 	.map_mem_usage = array_map_mem_usage,
 	.map_btf_id = &array_map_btf_ids[0],
diff --git a/tools/testing/selftests/bpf/prog_tests/cb_refs.c b/tools/testing/selftests/bpf/prog_tests/cb_refs.c
index c32c6dab49bc..645f065c21f5 100644
--- a/tools/testing/selftests/bpf/prog_tests/cb_refs.c
+++ b/tools/testing/selftests/bpf/prog_tests/cb_refs.c
@@ -12,8 +12,8 @@ struct {
 	const char *err_msg;
 } cb_refs_tests[] = {
 	{ "underflow_prog", "R1 type=scalar expected=ptr_, trusted_ptr_, rcu_ptr_" },
-	{ "leak_prog", "Unreleased reference id=4 alloc_insn=3" }, /* alloc_insn=3{2,3} */
-	{ "nested_cb", "Unreleased reference id=4 alloc_insn=2" }, /* alloc_insn=2{4,5} */
+	{ "leak_prog", "Unreleased reference id=5 alloc_insn=3" }, /* alloc_insn=3{2,3} */
+	{ "nested_cb", "Unreleased reference id=5 alloc_insn=2" }, /* alloc_insn=2{4,5} */
 	{ "non_cb_transfer_ref", "Unreleased reference id=4 alloc_insn=1" }, /* alloc_insn=1{1,2} */
 };
 
-- 
2.55.0

[PATCH bpf-next v1 6/6] selftests/bpf: Cover callback-frame map key lifetime

[Ihor Solodrai]

Parking the key in callback_ctx and dereferencing it after the iteration
is rejected for array and percpu-array maps, whose key lives in
bpf_for_each_array_elem()'s frame.

Two cases must keep verifying, and are the reason the declaration is not
in the shared map_set_for_each_callback_args(): the same shape over a
hash map, whose key points into the element, and parking the element
value, which lives until map teardown.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 .../bpf/progs/verifier_iterating_callbacks.c  | 91 +++++++++++++++++++
 1 file changed, 91 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
index 1fbcc5228306..2e0c56888953 100644
--- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
+++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
@@ -9,6 +9,20 @@ struct {
 	__type(value, __u64);
 } map SEC(".maps");
 
+struct {
+	__uint(type, BPF_MAP_TYPE_PERCPU_ARRAY);
+	__uint(max_entries, 8);
+	__type(key, __u32);
+	__type(value, __u64);
+} percpu_map SEC(".maps");
+
+struct {
+	__uint(type, BPF_MAP_TYPE_HASH);
+	__uint(max_entries, 8);
+	__type(key, __u32);
+	__type(value, __u64);
+} hash_map SEC(".maps");
+
 struct {
 	__uint(type, BPF_MAP_TYPE_USER_RINGBUF);
 	__uint(max_entries, 8);
@@ -800,4 +814,81 @@ __naked void check_add_const_regsafe_off(void)
 	: __clobber_common);
 }
 
+struct key_ctx {
+	__u32 *key;
+};
+
+static long park_key_cb(struct bpf_map *map, __u32 *key, __u64 *value,
+			void *context)
+{
+	struct key_ctx *c = context;
+
+	c->key = key;
+	return 0;
+}
+
+/* bpf_for_each_array_elem() passes a key from its own stack frame. */
+SEC("?raw_tp")
+__failure __msg("invalid mem access 'scalar'")
+int array_park_map_key(void *ctx)
+{
+	struct key_ctx c = {};
+
+	bpf_for_each_map_elem(&map, park_key_cb, &c, 0);
+	if (c.key)
+		return *c.key;
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __msg("invalid mem access 'scalar'")
+int percpu_array_park_map_key(void *ctx)
+{
+	struct key_ctx c = {};
+
+	bpf_for_each_map_elem(&percpu_map, park_key_cb, &c, 0);
+	if (c.key)
+		return *c.key;
+	return 0;
+}
+
+/* A hash key points into the element, which outlives the callback. */
+SEC("?raw_tp")
+__success
+int hash_park_map_key(void *ctx)
+{
+	struct key_ctx c = {};
+
+	bpf_for_each_map_elem(&hash_map, park_key_cb, &c, 0);
+	if (c.key)
+		return *c.key;
+	return 0;
+}
+
+struct value_ctx {
+	__u64 *value;
+};
+
+static long park_value_cb(struct bpf_map *map, __u32 *key, __u64 *value,
+			  void *context)
+{
+	struct value_ctx *c = context;
+
+	c->value = value;
+	return 0;
+}
+
+/* Only the key is frame-scoped; the element lives until map teardown. */
+SEC("?raw_tp")
+__success
+int array_park_map_value(void *ctx)
+{
+	struct value_ctx c = {};
+
+	bpf_for_each_map_elem(&map, park_value_cb, &c, 0);
+	if (c.value)
+		return *c.value;
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.55.0

Re: [PATCH bpf-next v1 6/6] selftests/bpf: Cover callback-frame map key lifetime

[bot+bpf-ci]

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

> selftests/bpf: Cover callback-frame map key lifetime
>
> Parking the key in callback_ctx and dereferencing it after the iteration
> is rejected for array and percpu-array maps, whose key lives in
> bpf_for_each_array_elem()'s frame.
>
> Two cases must keep verifying, and are the reason the declaration is not
> in the shared map_set_for_each_callback_args(): the same shape over a
> hash map, whose key points into the element, and parking the element
> value, which lives until map teardown.

This isn't a bug, but would it read more precisely to split these - the
hash-map case as the reason the declaration is per-map-ops, and the
parked-value case as a guard that only the key was declared frame-scoped?

Looking at 6cd471d244f2, the placement decision is driven by the first case
(hash_park_map_key), since that's what would break if mark_frame_scoped_arg
for BPF_REG_2 moved into the shared map_set_for_each_callback_args. The
second case (array_park_map_value) parks R3 (PTR_TO_MAP_VALUE), which no
placement of a BPF_REG_2 declaration can affect.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> index 1fbcc5228306..2e0c56888953 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_iterating_callbacks.c

[ ... ]

> @@ -800,4 +814,81 @@ __naked void check_add_const_regsafe_off(void)
>  	: __clobber_common);
>  }
>
> +struct key_ctx {
> +	__u32 *key;
> +};
> +
> +static long park_key_cb(struct bpf_map *map, __u32 *key, __u64 *value,
> +			void *context)
> +{
> +	struct key_ctx *c = context;
> +
> +	c->key = key;
> +	return 0;
> +}
> +
> +/* bpf_for_each_array_elem() passes a key from its own stack frame. */
> +SEC("?raw_tp")
> +__failure __msg("invalid mem access 'scalar'")
> +int array_park_map_key(void *ctx)
> +{
> +	struct key_ctx c = {};
> +
> +	bpf_for_each_map_elem(&map, park_key_cb, &c, 0);
> +	if (c.key)
> +		return *c.key;
> +	return 0;
> +}
> +
> +SEC("?raw_tp")
> +__failure __msg("invalid mem access 'scalar'")
> +int percpu_array_park_map_key(void *ctx)
> +{
> +	struct key_ctx c = {};
> +
> +	bpf_for_each_map_elem(&percpu_map, park_key_cb, &c, 0);
> +	if (c.key)
> +		return *c.key;
> +	return 0;
> +}
> +
> +/* A hash key points into the element, which outlives the callback. */
> +SEC("?raw_tp")
> +__success
> +int hash_park_map_key(void *ctx)
> +{
> +	struct key_ctx c = {};
> +
> +	bpf_for_each_map_elem(&hash_map, park_key_cb, &c, 0);
> +	if (c.key)
> +		return *c.key;
> +	return 0;
> +}
> +
> +struct value_ctx {
> +	__u64 *value;
> +};
> +
> +static long park_value_cb(struct bpf_map *map, __u32 *key, __u64 *value,
> +			  void *context)
> +{
> +	struct value_ctx *c = context;
> +
> +	c->value = value;
> +	return 0;
> +}
> +
> +/* Only the key is frame-scoped; the element lives until map teardown. */
> +SEC("?raw_tp")
> +__success
> +int array_park_map_value(void *ctx)
> +{
> +	struct value_ctx c = {};
> +
> +	bpf_for_each_map_elem(&map, park_value_cb, &c, 0);
> +	if (c.value)
> +		return *c.value;
> +	return 0;
> +}
> +
>  char _license[] SEC("license") = "GPL";


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

Re: [PATCH bpf-next v1 2/6] bpf: Scope the bpf_user_ringbuf_drain() dynptr to its callback frame

[bot+bpf-ci]

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

> bpf: Scope the bpf_user_ringbuf_drain() dynptr to its callback frame
>
> bpf_user_ringbuf_drain() peeks a sample, initialises a struct
> bpf_dynptr_kern on its own stack, passes it to the callback, and calls
> __bpf_user_ringbuf_sample_release() as soon as the callback returns.
> Neither the descriptor nor the sample it describes is valid afterwards.
> set_user_ringbuf_callback_state() only gives the callback's R1 a type.
> The callback can therefore store the CONST_PTR_TO_DYNPTR register into
> callback_ctx, which points into a frame that outlives the call, and the
> program can use it after the drain returns. Three routes reach past the
> callback:
>   - the register itself, spillable since v7.3-rc1, which points at a
>     descriptor on reused kernel stack and gives the program an arbitrary
>     kernel read/write
>   - a bpf_dynptr_data() or bpf_dynptr_slice() result, which carries
>     the dynptr's id as its parent_id
>   - a bpf_dynptr_clone(), which is a by-value copy in the caller's
>     frame and can be sliced after the drain returns
> Declare R1 frame-scoped so all three are invalidated when the callback
> frame is popped.
> Reported-by: Nicholas Carlini <npc@anthropic.com>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

This isn't a bug, but should this include a Fixes: tag? The commit fixes a
security vulnerability (arbitrary kernel read/write via use-after-free).

  Fixes: 205715673844 ("bpf: Add bpf_user_ringbuf_drain() helper")


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

Re: [PATCH bpf-next v1 4/6] selftests/bpf: Cover the user ringbuf callback dynptr lifetime

[bot+bpf-ci]

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

> commit ffea6db696c062005bd2724d8b3888fa7eaa4768
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> Date:   Mon Sep 21 18:03:31 2026 -0700
>
>     selftests/bpf: Cover the user ringbuf callback dynptr lifetime
>
>     Add a rejection test for each route the bpf_user_ringbuf_drain() callback
>     dynptr can take out of its frame:
>
>       - the CONST_PTR_TO_DYNPTR register parked in callback_ctx
>       - a bpf_dynptr_data() slice
>       - a bpf_dynptr_slice() slice
>       - a bpf_dynptr_clone() written into the caller's frame
>       - a slice taken from that clone after the drain returns
>       - an inner drain's dynptr escaping into an outer callback
>
>     The first also checks that the diagnostic names the callback.
>
>     bpf_throw() from the callback reaches check_reference_leak() with the
>     frame reference still live, and is rejected afterwards by
>     check_max_stack_depth(). It matches that rejection in full, because
>     "bpf_throw" alone also matches the reference-leak wording the callback
>     must not produce.
>
>     Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

This isn't a bug, but the changelog says "The first" also checks the
diagnostic names the callback, yet four of the six programs match "the
callback that owned this value returned" - would it help to say that the
parked-dynptr/slice/nested routes all pin that wording and only the clone
routes differ?

> diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
> index 22503cf62e9f..86a0667ba348 100644
> --- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
> +++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c

[ ... ]

> diff --git a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
> index c0d0422b8030..a8d3acfa2bd2 100644
> --- a/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
> +++ b/tools/testing/selftests/bpf/progs/user_ringbuf_fail.c
> @@ -1,9 +1,11 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /* Copyright (c) 2022 Meta Platforms, Inc. and affiliates. */
>
> +#include <stdbool.h>
>  #include <linux/bpf.h>
>  #include <bpf/bpf_helpers.h>
>  #include "bpf_misc.h"
> +#include "bpf_kfuncs.h"
>
>  char _license[] SEC("license") = "GPL";

[ ... ]

> @@ -243,3 +245,144 @@ int user_ringbuf_callback_const_ptr_to_dynptr_reg_off(void *ctx)
>  			       callback_adjust_bpf_dynptr_reg_off, NULL, 0);
>  	return 0;
>  }
> +
> +/* The sample goes back to the producer as soon as the callback returns. */
> +struct dynptr_ctx {
> +	struct bpf_dynptr *saved;
> +};
> +
> +static long callback_park_dynptr(struct bpf_dynptr *dynptr, void *context)
> +{
> +	struct dynptr_ctx *c = context;
> +
> +	c->saved = dynptr;
> +	return 0;
> +}

[ ... ]

> +static long callback_park_inner(struct bpf_dynptr *dynptr, void *context)
> +{
> +	struct dynptr_ctx *c = context;
> +
> +	c->saved = dynptr;
> +	return 0;
> +}

This isn't a bug, but callback_park_inner looks identical to
callback_park_dynptr - could the nested test pass callback_park_dynptr to
the inner drain instead, the way callback_park_clone is already shared by
the two clone tests?

> +
> +/* An inner drain's dynptr must not escape into the outer callback either. */
> +static long callback_park_outer(struct bpf_dynptr *dynptr, void *context)
> +{
> +	struct dynptr_ctx inner = {};
> +	char buf[8] = {};
> +
> +	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_inner, &inner, 0);
> +	if (inner.saved)
> +		bpf_dynptr_read(buf, sizeof(buf), inner.saved, 0, 0);
> +	return buf[0] ? 1 : 0;
> +}
> +
> +SEC("?raw_tp")
> +__failure __msg("the callback that owned this value returned")
> +int user_ringbuf_callback_nested_park_inner(void *ctx)
> +{
> +	bpf_user_ringbuf_drain(&user_ringbuf, callback_park_outer, NULL, 0);
> +	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/35674974944

Re: [PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[Alexei Starovoitov]

On Mon, Sep 21, 2026 at 06:03 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:

> Neither has a local fix: both arguments point into a helper's own
> frame, with nothing longer-lived to anchor them to.

It's the same problem as a pointer to callee's stack.
check_stack_write_fixed_off() deals with it like this:
  if (state != cur && reg->type == PTR_TO_STACK) {
          verbose(env, "cannot spill pointers to stack into stack frame of the caller\n");
          return -EINVAL;
  }
CONST_PTR_TO_DYNPTR wasn't spillable before 7.3, so nothing depends
on parking it in the caller's frame. Reject it there too ?
and then no need for REF_TYPE_FRAME complexity?

Re: [PATCH bpf-next v1 1/6] bpf: Introduce REF_TYPE_FRAME in the verifier

[bot+bpf-ci]

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

> commit d7733bc00f8457babb8d4c38adaf2babe3427069
> Author: Ihor Solodrai <ihor.solodrai@linux.dev>
> Date:   Mon Sep 21 18:03:28 2026 -0700
>
>     bpf: Introduce REF_TYPE_FRAME in the verifier
>
>     A callback-calling helper can pass its callback a pointer that is only
>     valid for the duration of the call. The verifier just gives such an
>     argument a register type, which allows the callback to park the value,
>     or something derived from it, and make it outlive the frame. The BPF
>     program then can reuse it after the helper returns.
>
>     Invalidating a value together with everything derived from it is what
>     release_reference() already does, walking reg->parent_id across every
>     frame and stack slot. What is missing is a type of reference that is
>     not an object the program acquired and releases.
>
>     Introduce REF_TYPE_FRAME: a reference owned by a callee frame.
>     A set_callee_state_fn can declare an argument frame-scoped, and
>     setup_func_entry() turns the declaration into a reference, and
>     prepare_func_exit() drops it when the frame is popped, invalidating
>     the argument and everything derived from it through the existing walk.
>
>     Keep the new type invisible to find_reference_state(). release_reg()
>     and ref_convert_owning_non_owning() look up purely by id and could
>     otherwise destroy the anchor.
>
>     Fix up a few pre-existing comments while at it.
>
>     Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

This isn't a bug, but the changelog says release_reference() "already
does" this walk, though the patch also adds "&& reg->id != id" to it.
Would it be worth a sentence saying why - that the frame anchor has
reg->id == reg->parent_id and the DFS would otherwise re-push the same
id forever?

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index fd22db8bc6c5..d849e4873417 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3118,6 +3118,7 @@ int bpf_iter_map_fill_link_info(const struct bpf_iter_aux_info *aux,
>  int map_set_for_each_callback_args(struct bpf_verifier_env *env,
>  				   struct bpf_func_state *caller,
>  				   struct bpf_func_state *callee);
> +void mark_frame_scoped_arg(struct bpf_func_state *callee, u32 regno);
>
>  int bpf_percpu_hash_copy(struct bpf_map *map, void *key, void *value, u64 flags);
>  int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value, u64 flags);
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 92f528c45605..51c310ea4e82 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h

[ ... ]

> @@ -387,6 +387,11 @@ struct bpf_func_state {
>  	u32 callback_depth;
>  	/* Instructions processed in this frame and callees on the current path. */
>  	u32 insns_subtotal;
> +	/*
> +	 * Set for arguments valid until the frame is popped.
> +	 * Consumed by setup_func_entry().
> +	 */
> +	u16 frame_scoped_args;
>
>  	/* The following fields should be last. See copy_func_state() */
>  	/* The state of the stack. Each element of the array describes BPF_REG_SIZE

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index d62c0f74cff5..a0a9d3d18f63 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1474,14 +1474,13 @@ static int grow_stack_arg_slots(struct bpf_verifier_env *env,
>  	return 0;
>  }
>
> -/* Acquire a pointer id from the env and update the state->refs to include
> - * this new pointer reference.
> - * On success, returns a valid pointer id to associate with the register
> - * On failure, returns a negative errno.
> +/* Append an entry to @state->refs and record the instruction that created it.
> + * The caller fills in the type and the id.
> + * On success, returns the new entry. On failure, returns NULL.
>   */
> -static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> +static struct bpf_reference_state *__acquire_reference_state(struct bpf_verifier_state *state,
> +							     int insn_idx)
>  {
> -	struct bpf_verifier_state *state = env->cur_state;
>  	int new_ofs = state->acquired_refs;
>  	int err;
>

This isn't a bug, but the comment is rewritten in the patch with
multi-line text starting on the same line as the opening "/*", while the
other three multi-line comments added by this commit use the modern
kernel style with "/*" alone on the first line. Could this one match the
others for consistency?

> @@ -1493,6 +1492,12 @@ static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_e
>  	return &state->refs[new_ofs];
>  }
>
> +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env,
> +							   int insn_idx)
> +{
> +	return __acquire_reference_state(env->cur_state, insn_idx);
> +}
> +
>  static int acquire_reference(struct bpf_verifier_env *env, int insn_idx, int parent_id)
>  {
>  	struct bpf_reference_state *s;
> @@ -1507,6 +1512,31 @@ static int acquire_reference(struct bpf_verifier_env *env, int insn_idx, int par
>  	return s->id;
>  }
>
> +/* Acquire a reference owned by frame @frameno of @state */
> +static int acquire_frame_reference(struct bpf_verifier_env *env, struct bpf_verifier_state *state,
> +				   int insn_idx, u32 frameno)
> +{
> +	struct bpf_reference_state *s;
> +
> +	s = __acquire_reference_state(state, insn_idx);
> +	if (!s)
> +		return -ENOMEM;
> +	s->type = REF_TYPE_FRAME;
> +	s->id = ++env->id_gen;
> +	s->frameno = frameno;
> +	return s->id;
> +}
> +
> +/*
> + * Declare that @regno in @callee holds a value that stops being valid once the
> + * frame is popped. setup_func_entry() turns each declaration into a frame-owned
> + * reference.
> + */
> +void mark_frame_scoped_arg(struct bpf_func_state *callee, u32 regno)
> +{
> +	callee->frame_scoped_args |= BIT(regno);
> +}
> +

[ ... ]

> @@ -10175,7 +10205,7 @@ static int release_reference(struct bpf_verifier_env *env, int id)
>  				continue;
>
>  			/* Free objects derived from the current object */
> -			if (reg->parent_id == id) {
> +			if (reg->parent_id == id && reg->id != id) {
>  				err = idstack_push(idstack, reg->id);
>  				if (err)
>  					return err;

[ ... ]

> @@ -10333,6 +10392,30 @@ static int setup_func_entry(struct bpf_verifier_env *env, int subprog, int calls
>  	if (err)
>  		goto err_out;
>
> +	scoped_args = callee->frame_scoped_args;
> +	callee->frame_scoped_args = 0;
> +	for (regno = 0; regno < MAX_BPF_REG; regno++) {
> +		int id;
> +
> +		if (!(scoped_args & BIT(regno)))
> +			continue;
> +
> +		id = acquire_frame_reference(env, state, callsite, callee->frameno);
> +		if (id < 0) {
> +			err = id;
> +			goto err_out;
> +		}
> +		/*
> +		 * The value is its own lifetime anchor: there is no associated
> +		 * object to borrow from, only the frame. parent_id = id here
> +		 * covers both possible derived references:
> +		 *  - through the id (e.g. dynptr slice)
> +		 *  - through parent_id (e.g. dynptr clone)
> +		 */
> +		callee->regs[regno].id = id;
> +		callee->regs[regno].parent_id = id;
> +	}
> +
>  	/* only increment it after check_reg_arg() finished */
>  	state->curframe++;

[ ... ]


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

Re: [PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[Kumar Kartikeya Dwivedi]

On Tue Sep 22, 2026 at 3:55 AM CEST, Alexei Starovoitov wrote:
> On Mon, Sep 21, 2026 at 06:03 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
>
>> Neither has a local fix: both arguments point into a helper's own
>> frame, with nothing longer-lived to anchor them to.
>
> It's the same problem as a pointer to callee's stack.
> check_stack_write_fixed_off() deals with it like this:
>   if (state != cur && reg->type == PTR_TO_STACK) {
>           verbose(env, "cannot spill pointers to stack into stack frame of the caller\n");
>           return -EINVAL;
>   }
> CONST_PTR_TO_DYNPTR wasn't spillable before 7.3, so nothing depends
> on parking it in the caller's frame. Reject it there too ?
> and then no need for REF_TYPE_FRAME complexity?

+1, will be much simpler fix.

Re: [PATCH bpf-next v1 0/6] bpf: Scope callback arguments to their frame

[Ihor Solodrai]

On 2026-09-21 6:55 p.m., Alexei Starovoitov wrote:
> On Mon, Sep 21, 2026 at 06:03 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
> 
>> Neither has a local fix: both arguments point into a helper's own
>> frame, with nothing longer-lived to anchor them to.
> 
> It's the same problem as a pointer to callee's stack.
> check_stack_write_fixed_off() deals with it like this:
>    if (state != cur && reg->type == PTR_TO_STACK) {
>            verbose(env, "cannot spill pointers to stack into stack frame of the caller\n");
>            return -EINVAL;
>    }
> CONST_PTR_TO_DYNPTR wasn't spillable before 7.3, so nothing depends
> on parking it in the caller's frame. Reject it there too ?
> and then no need for REF_TYPE_FRAME complexity?

If we focus on the nasty bpf_user_ringbuf_drain() bug specifically,
then yes, check_stack_write_fixed_off() change patches it:

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d62c0f74cff5..f261423e9282 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3668,7 +3668,8 @@ static int check_stack_write_fixed_off(struct 
bpf_verifier_env *env,
                         verbose(env, "invalid size of register spill\n");
                         return -EACCES;
                 }
-               if (state != cur && reg->type == PTR_TO_STACK) {
+               if (state != cur && (reg->type == PTR_TO_STACK ||
+                                    reg->type == CONST_PTR_TO_DYNPTR)) {
                         verbose(env, "cannot spill pointers to stack 
into stack frame of the caller\n");
                         return -EINVAL;
                 }

However it doesn't cover some of the new test cases:
   - user_ringbuf_callback_park_data_slice
   - user_ringbuf_callback_park_kfunc_slice
   - user_ringbuf_callback_park_clone
   - user_ringbuf_callback_park_clone_then_slice

(not counting the diag message diff)

The original suggestion that came with the bug report was a
cb_dynptr_id field in bpf_func_state set up in
set_user_ringbuf_callback_state() and read in prepare_func_exit() to
release it there.

The cb_dynptr_id seemed way too specific, I didn't like it. So I've
tried to figure out a feasible generalization of the problem, and came
to "verifier can't track a lifetime of a ref tied to a frame", and
then to this series.

I think we need to decide whether the REF_TYPE_FRAME is a useful
mechanism in principle, and whether it's sufficiently generic. It at
least covers the cases in this series and more.

For example AI also flagged for me parking the vma argument
(PTR_TO_BTF_ID) of the bpf_find_vma() callback. It's low severity,
which is why I excluded that from the series, but "reject a reg type"
wouldn't work there AFAIU (we would break many legitimate programs).

Opinions?