BPF List
 help / color / mirror / Atom feed
From: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>
To: "Ihor Solodrai" <ihor.solodrai@linux.dev>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Eduard Zingerman" <eddyz87@gmail.com>,
	"Kumar Kartikeya Dwivedi" <memxor@gmail.com>
Cc: "Amery Hung" <ameryhung@gmail.com>,
	"Emil Tsalapatis" <emil@etsalapatis.com>,
	"Nicholas Carlini" <npc@anthropic.com>, <bpf@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <kernel-team@meta.com>
Subject: Re: [PATCH bpf-next v2 00/11] bpf: Scope callback arguments to their frame
Date: Fri, 16 Oct 2026 16:42:39 +0000	[thread overview]
Message-ID: <DM6EVHTVM3IR.3RCFLKV9WC80J@gmail.com> (raw)
In-Reply-To: <20261009202759.50520-1-ihor.solodrai@linux.dev>

On Fri, Oct 09, 2026 at 01:27 PM Ihor Solodrai <ihor.solodrai@linux.dev> wrote:
> Introduce REF_TYPE_FRAME for this use case: a reference owned by a
> callee frame, dropped when the frame is popped. Tie each argument to
> this reference so callback return invalidates it and its descendants.

Another idea...
The id is owned by the frame. Can we keep it in bpf_func_state
like diag_frame_id ?
release_reference() already takes ids that are not in refs[].
See unmark_stack_slots_dynptr().
Clones copy parent_id and slices hang off dynptr's id, so both are
reached.
All setters have callee already, so no need for patches 1-3,
REF_TYPE_FRAME, frameno in refs[], the reg that is its own parent, etc.
Something like this (compile tested only):

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 54144372281c..4f82b86cdfe2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3185,6 +3185,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_verifier_env *env, 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 112ca1511da4..8865ece94ef4 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -364,6 +364,8 @@ struct bpf_func_state {
 	 * reused after returns, while this ID is preserved across state clones.
 	 */
 	u32 diag_frame_id;
+	/* parent_id of the args that are gone when this frame returns */
+	u32 frame_id;
 	/* subprog number == index within subprog_info
 	 * zero == main subprog
 	 */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 0fe9afd4a591..3376cc81e5d9 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -855,6 +855,16 @@ 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)
+{
+	map_set_for_each_callback_args(env, caller, callee);
+	/* the key is on the stack of bpf_for_each_array_elem() */
+	mark_frame_scoped_arg(env, 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 +885,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 +910,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/kernel/bpf/states.c b/kernel/bpf/states.c
index 953700822fa7..43d6682037fe 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -995,6 +995,9 @@ static bool func_states_equal(struct bpf_verifier_env *env, struct bpf_func_stat
 	if (!old->no_stack_arg_load && cur->no_stack_arg_load)
 		return false;
 
+	if (!check_ids(old->frame_id, cur->frame_id, &env->idmap_scratch))
+		return false;
+
 	for (i = 0; i < MAX_BPF_REG; i++)
 		if (((1 << i) & live_regs) &&
 		    !regsafe(env, &old->regs[i], &cur->regs[i],
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ec136cf377c1..bc12df55d2e2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11465,6 +11465,14 @@ static int check_func_callx(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	return check_static_func_call(env, subprog, insn_idx);
 }
 
+/* The arg points into the stack frame of the helper that calls the callback */
+void mark_frame_scoped_arg(struct bpf_verifier_env *env, struct bpf_func_state *callee, u32 regno)
+{
+	if (!callee->frame_id)
+		callee->frame_id = ++env->id_gen;
+	callee->regs[regno].parent_id = callee->frame_id;
+}
+
 int map_set_for_each_callback_args(struct bpf_verifier_env *env,
 				   struct bpf_func_state *caller,
 				   struct bpf_func_state *callee)
@@ -11631,6 +11639,7 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
 	 */
 	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_frame_scoped_arg(env, callee, BPF_REG_1);
 	callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3];
 
 	/* unused */
@@ -11885,6 +11894,11 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
 					     callee->allocated_stack / BPF_REG_SIZE - 1);
 	if (err)
 		return err;
+	if (callee->frame_id) {
+		err = release_reference(env, callee->frame_id);
+		if (err)
+			return err;
+	}
 
 	/* for callbacks like bpf_loop or bpf_for_each_map_elem go back to callsite,
 	 * there function call logic would reschedule callback visit. If iteration

Patches 5 and 6 are fine on their own. Pls send them separately.

pw-bot: cr

      parent reply	other threads:[~2026-10-16 16:42 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-09 20:27 [PATCH bpf-next v2 00/11] bpf: Scope callback arguments to their frame Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 01/11] bpf: Set up callee state outside of setup_func_entry() Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 02/11] bpf: Pass the owning verifier state to callback setters Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 03/11] bpf: Append complete reference states Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 04/11] bpf: Separate reference removal from descendant invalidation Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 05/11] bpf: Share map-backed callback register setup Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 06/11] bpf: Rely on callback frame initialization defaults Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 07/11] bpf: Introduce REF_TYPE_FRAME in the verifier Ihor Solodrai
2026-10-09 20:48   ` sashiko-bot
2026-10-10  3:26     ` Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 08/11] bpf: Scope the bpf_user_ringbuf_drain() dynptr to its callback frame Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 09/11] selftests/bpf: Cover the user ringbuf callback dynptr lifetime Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 10/11] bpf: Scope the bpf_for_each_map_elem() array key to the callback frame Ihor Solodrai
2026-10-09 20:27 ` [PATCH bpf-next v2 11/11] selftests/bpf: Cover callback-frame map key lifetime Ihor Solodrai
2026-10-16 16:42 ` Alexei Starovoitov [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6EVHTVM3IR.3RCFLKV9WC80J@gmail.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=ameryhung@gmail.com \
    --cc=andrii@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=emil@etsalapatis.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=memxor@gmail.com \
    --cc=npc@anthropic.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox