[PATCH bpf-next v2 0/5] Object relationship tracking refactor followup

[PATCH bpf-next v2 0/5] Object relationship tracking refactor followup

[Amery Hung]

Hi,

The main patchset refactoring object relationship tracking in the
verifier has landed and this is a followup that addresses the remaining
feedback in v6 [0].

[0]
https://lore.kernel.org/bpf/20260529014936.2811085-1-ameryhung@gmail.com/

Amery Hung (5):
  bpf: Fix dead error check on acquire_reference() in check_kfunc_call
  bpf: Check acquire_reference() error for "__ref" struct_ops arguments
  bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
  bpf: Remove WARN_ON_ONCE in check_ids()
  selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test

 kernel/bpf/states.c                               | 11 +++++++++--
 kernel/bpf/verifier.c                             | 15 +++++++++++----
 .../selftests/bpf/progs/file_reader_fail.c        |  8 ++++----
 3 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.53.0-Meta

[PATCH bpf-next v2 1/5] bpf: Fix dead error check on acquire_reference() in check_kfunc_call

[Amery Hung]

acquire_reference() returns a signed int that may be a negative errno
but was converted to unsigned, which makes the subsequent error check
deadcode. Fix it by declaring 'id' as int so the error path is taken
correctly.

Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/verifier.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8ed484cb1a8a..6446db9628ae 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12817,9 +12817,10 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	struct bpf_kfunc_call_arg_meta meta;
 	struct bpf_insn_aux_data *insn_aux;
 	int err, insn_idx = *insn_idx_p;
-	u32 i, nargs, ptr_type_id, id;
 	const struct btf_param *args;
+	u32 i, nargs, ptr_type_id;
 	struct btf *desc_btf;
+	int id;
 
 	/* skip for now, but return error when we find this in fixup_kfunc_call */
 	if (!insn->imm)
-- 
2.53.0-Meta

[PATCH bpf-next v2 2/5] bpf: Check acquire_reference() error for "__ref" struct_ops arguments

[Amery Hung]

When acquiring references for struct_ops program arguments tagged with
"__ref", the return value of acquire_reference() was stored directly
into u32 ctx_arg_info[i].ref_id without checking for failure.
acquire_reference() returns -ENOMEM when acquire_reference_state() fails
to allocate, so the error was silently stored as a ref_id instead of
aborting verification. Fix it by checking the return.

Fixes: a687df2008f6 ("bpf: Support getting referenced kptr from struct_ops argument")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/verifier.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6446db9628ae..2a18b8aba14d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18362,9 +18362,15 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
 
 	/* Acquire references for struct_ops program arguments tagged with "__ref" */
 	if (!subprog && env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
-		for (i = 0; i < aux->ctx_arg_info_size; i++)
-			aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
-						      acquire_reference(env, 0, 0) : 0;
+		for (i = 0; i < aux->ctx_arg_info_size; i++) {
+			int id;
+
+			id = aux->ctx_arg_info[i].refcounted ? acquire_reference(env, 0, 0) : 0;
+			if (id < 0)
+				return id;
+
+			aux->ctx_arg_info[i].ref_id = id;
+		}
 	}
 
 	ret = do_check(env);
-- 
2.53.0-Meta

[PATCH bpf-next v2 3/5] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR

[Amery Hung]

refsafe() compared each reference's id and type but not its parent_id,
so two states whose PTR references differ only in the parent object they
were derived from could be wrongly treated as equivalent and pruned. Fix
it by checking parent_id too.

Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/states.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 5945956a7573..06d9ae24f006 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -890,6 +890,9 @@ static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *c
 			return false;
 		switch (old->refs[i].type) {
 		case REF_TYPE_PTR:
+			if (!check_ids(old->refs[i].parent_id, cur->refs[i].parent_id, idmap))
+				return false;
+			break;
 		case REF_TYPE_IRQ:
 			break;
 		case REF_TYPE_LOCK:
-- 
2.53.0-Meta

[PATCH bpf-next v2 4/5] bpf: Remove WARN_ON_ONCE in check_ids()

[Amery Hung]

check_ids() warned when it ran out of idmap slots, assuming this was
impossible because the slots are bounded by the number of registers and
stack slots. That assumption no longer holds: referenced dynptrs acquire
an intermediate reference that lives in refs[] but is not backed by any
register or stack slot [0], so a program can accumulate more reference
ids than the idmap can hold and exhaust it.

Exhaustion is fine for verification correctness. check_ids() already
returns false, which makes the states compare as not equivalent and
prevents unsound pruning. The only effect of the WARN_ON_ONCE() is log
noise, or a panic under panic_on_warn. Drop the warning and keep
returning false.

[0] 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 kernel/bpf/states.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 06d9ae24f006..32f346ce3ffc 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
@@ -343,8 +343,12 @@ static bool check_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
 		return true;
 	}
 
-	/* We ran out of idmap slots, which should be impossible */
-	WARN_ON_ONCE(1);
+	/*
+	 * idmap slots are bounded by the number of registers and stack slots.
+	 * Since referenced dynptrs acquire intermediate references that do
+	 * not live in either, so the map can be exhausted. Since it is unlikely,
+	 * fail the verification by treating the states as not equivalent.
+	 */
 	return false;
 }
 
-- 
2.53.0-Meta

[PATCH bpf-next v2 5/5] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test

[Amery Hung]

use_file_dynptr_slice_after_put_file() reads the dynptr via
bpf_dynptr_data(), which always returns NULL for a read-only file
dynptr, making the example confusing. Switch to bpf_dynptr_slice(), the
correct read API for file dynptrs, and read (rather than write) the slice
since it is read-only. The test still fails as expected.

Signed-off-by: Amery Hung <ameryhung@gmail.com>
---
 tools/testing/selftests/bpf/progs/file_reader_fail.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/file_reader_fail.c b/tools/testing/selftests/bpf/progs/file_reader_fail.c
index d5fae5e4cf9a..3bb9e2612f8f 100644
--- a/tools/testing/selftests/bpf/progs/file_reader_fail.c
+++ b/tools/testing/selftests/bpf/progs/file_reader_fail.c
@@ -87,7 +87,8 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
 	struct task_struct *task = bpf_get_current_task_btf();
 	struct file *file = bpf_get_task_exe_file(task);
 	struct bpf_dynptr dynptr;
-	char *data;
+	char buf[1];
+	const char *data;
 
 	if (!file)
 		return 0;
@@ -95,15 +96,14 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
 	if (bpf_dynptr_from_file(file, 0, &dynptr))
 		goto out;
 
-	data = bpf_dynptr_data(&dynptr, 0, 1);
+	data = bpf_dynptr_slice(&dynptr, 0, buf, sizeof(buf));
 	if (!data)
 		goto out;
 
 	/* this should fail - file dynptr should be discarded first to prevent resource leak */
 	bpf_put_file(file);
 
-	*data = 'x';
-	return 0;
+	return data[0];
 
 out:
 	bpf_dynptr_file_discard(&dynptr);
-- 
2.53.0-Meta

Re: [PATCH bpf-next v2 2/5] bpf: Check acquire_reference() error for "__ref" struct_ops arguments

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct return on `acquire_reference()` failure in `do_check_common()` bypasses cleanup and leaks `env->cur_state`.
--

commit 255de8a669b1a40ba719e137d38c8f3f04ff00ca
Author: Amery Hung <ameryhung@gmail.com>

bpf: Check acquire_reference() error for "__ref" struct_ops arguments

This commit adds an error check for acquire_reference() when handling
struct_ops program arguments tagged with "__ref". It stores the error
return value and aborts verification if acquire_reference_state() fails to
allocate memory.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a741bf4479318..5a69af5690891 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18363,9 +18363,15 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
>  
>  	/* Acquire references for struct_ops program arguments tagged with "__ref" */
>  	if (!subprog && env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
> -		for (i = 0; i < aux->ctx_arg_info_size; i++)
> -			aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
> -						      acquire_reference(env, 0, 0) : 0;
> +		for (i = 0; i < aux->ctx_arg_info_size; i++) {
> +			int id;
> +
> +			id = aux->ctx_arg_info[i].refcounted ? acquire_reference(env, 0, 0) : 0;
> +			if (id < 0)
> +				return id;

[Severity: High]
Does this direct return bypass the cleanup path and leak env->cur_state?

Earlier in do_check_common(), env->cur_state is allocated via kzalloc_obj().
Other error paths in this function set the return variable and jump to the
out label to call free_states(env) for proper cleanup.

Should this be updated to set ret = id and goto out instead of returning
directly?

> +
> +			aux->ctx_arg_info[i].ref_id = id;
> +		}
>  	}
>  
>  	ret = do_check(env);

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

Re: [PATCH bpf-next v2 5/5] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/progs/file_reader_fail.c b/tools/testing/selftests/bpf/progs/file_reader_fail.c
> index d5fae5e4cf9a..3bb9e2612f8f 100644
> --- a/tools/testing/selftests/bpf/progs/file_reader_fail.c
> +++ b/tools/testing/selftests/bpf/progs/file_reader_fail.c
> @@ -87,7 +87,8 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
>  	struct task_struct *task = bpf_get_current_task_btf();
>  	struct file *file = bpf_get_task_exe_file(task);
>  	struct bpf_dynptr dynptr;
> -	char *data;
> +	char buf[1];
> +	const char *data;
>
>  	if (!file)
>  		return 0;
> @@ -95,15 +96,14 @@ int use_file_dynptr_slice_after_put_file(void *ctx)
>  	if (bpf_dynptr_from_file(file, 0, &dynptr))
>  		goto out;
>
> -	data = bpf_dynptr_data(&dynptr, 0, 1);
> +	data = bpf_dynptr_slice(&dynptr, 0, buf, sizeof(buf));

A note from the earlier review on v1: an automated review bot suggested
initializing the stack buffer here, e.g.

	char buf[1] = {0};

worrying that the uninitialized 'buf' passed to bpf_dynptr_slice() could
make the verifier reject the program early with an invalid stack read,
which would bypass the intended reference-leak check.

This looks like a false positive, since the buf argument to
bpf_dynptr_slice() is a destination the helper writes into, so
uninitialized stack memory should be valid there, and the v1 patch
already carried an Acked-by from Eduard Zingerman.

Was this comment considered, and is the test still failing for the
intended reference-leak reason rather than an early verifier rejection?


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

Re: [PATCH bpf-next v2 4/5] bpf: Remove WARN_ON_ONCE in check_ids()

[bot+bpf-ci]

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

> bpf: Remove WARN_ON_ONCE in check_ids()
>
> check_ids() warned when it ran out of idmap slots, assuming this was
> impossible because the slots are bounded by the number of registers and
> stack slots. That assumption no longer holds: referenced dynptrs acquire
> an intermediate reference that lives in refs[] but is not backed by any
> register or stack slot [0], so a program can accumulate more reference
> ids than the idmap can hold and exhaust it.
>
> Exhaustion is fine for verification correctness. check_ids() already
> returns false, which makes the states compare as not equivalent and
> prevents unsound pruning. The only effect of the WARN_ON_ONCE() is log
> noise, or a panic under panic_on_warn. Drop the warning and keep
> returning false.
>
> [0] 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>

This removes a WARN_ON_ONCE() that can panic under panic_on_warn, and
the commit body explains the idmap exhaustion only became reachable once
referenced dynptrs began acquiring an intermediate reference in refs[]
that is not backed by a register or stack slot.

Should this carry a Fixes: tag pointing at the commit that introduced
that behavior, which the body already cites as [0]?

  Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")

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

Re: [PATCH bpf-next v2 2/5] bpf: Check acquire_reference() error for "__ref" struct_ops arguments

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a741bf447931..5a69af569089 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18363,9 +18363,15 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
>
>  	/* Acquire references for struct_ops program arguments tagged with "__ref" */
>  	if (!subprog && env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
> -		for (i = 0; i < aux->ctx_arg_info_size; i++)
> -			aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
> -						      acquire_reference(env, 0, 0) : 0;
> +		for (i = 0; i < aux->ctx_arg_info_size; i++) {
> +			int id;
> +
> +			id = aux->ctx_arg_info[i].refcounted ? acquire_reference(env, 0, 0) : 0;
> +			if (id < 0)
> +				return id;

Does this direct return bypass the cleanup path and leak env->cur_state?

Earlier in do_check_common(), env->cur_state is allocated via kzalloc_obj().
Other error paths in this function set the return variable and jump to the
out label to call free_states(env) for proper cleanup.

Should this be updated to set ret = id and goto out instead of returning
directly?

> +
> +			aux->ctx_arg_info[i].ref_id = id;
> +		}
>  	}
>
>  	ret = do_check(env);


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

Re: [PATCH bpf-next v2 3/5] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 5945956a7573..06d9ae24f006 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -890,6 +890,9 @@ static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *c
>  			return false;
>  		switch (old->refs[i].type) {
>  		case REF_TYPE_PTR:
> +			if (!check_ids(old->refs[i].parent_id, cur->refs[i].parent_id, idmap))
> +				return false;
> +			break;
>  		case REF_TYPE_IRQ:
>  			break;
>  		case REF_TYPE_LOCK:

This change now feeds parent_id values through check_ids() in addition to
register and stack slot ids.

In the v1 discussion, Eduard Zingerman noted that since not all ids
correspond to register or stack slots anymore, the WARN_ON_ONCE(1) in
check_ids() on idmap exhaustion should be removed:

kernel/bpf/states.c:check_ids() {
	...
	/* Reached the end of known mappings; haven't seen this id before */
	if (idmap->cnt < BPF_ID_MAP_SIZE) {
		map[idmap->cnt].old = old_id;
		map[idmap->cnt].cur = cur_id;
		idmap->cnt++;
		return true;
	}

	/* We ran out of idmap slots, which should be impossible */
	WARN_ON_ONCE(1);
	return false;
}

The concern raised by the CI and Sashiko bots was that a BPF program
leaking many references could exhaust BPF_ID_MAP_SIZE and trigger this
warning, which under panic_on_warn becomes a denial of service. Returning
false on exhaustion was agreed to be acceptable; only the warning was
questioned.

The reply on the thread acknowledged this ("Ack. I will drop
WARN_ON_ONCE(1)."), but the WARN_ON_ONCE(1) is still present in this commit.

Was the WARN_ON_ONCE(1) removal intended for a separate patch in the
series, or should it be folded in here?

Reference:
https://lore.kernel.org/bpf/6d86c2fae9be63eb110ebd5b3fc19bc4cf1de2dc.camel@gmail.com/


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

Re: [PATCH bpf-next v2 1/5] bpf: Fix dead error check on acquire_reference() in check_kfunc_call

[bot+bpf-ci]

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7d27ba396d32..a741bf447931 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12817,9 +12817,10 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	struct bpf_kfunc_call_arg_meta meta;
>  	struct bpf_insn_aux_data *insn_aux;
>  	int err, insn_idx = *insn_idx_p;
> -	u32 i, nargs, ptr_type_id, id;
>  	const struct btf_param *args;
> +	u32 i, nargs, ptr_type_id;
>  	struct btf *desc_btf;
> +	int id;
>
>  	/* skip for now, but return error when we find this in fixup_kfunc_call */
>  	if (!insn->imm)

This patch fixes the missing error check in check_kfunc_call(), but
could a similar issue exist in do_check_common() when initializing
struct_ops program arguments?

Looking at do_check_common(), there's a similar pattern where
acquire_reference() is called:

kernel/bpf/verifier.c:do_check_common() {
    ...
    for (i = 0; i < aux->ctx_arg_info_size; i++)
        aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
                                      acquire_reference(env, 0, 0) : 0;
    ...
}

Since acquire_reference() can return a negative error like -ENOMEM,
assigning it directly to aux->ctx_arg_info[i].ref_id (which is a u32)
without checking for errors would swallow the error code.

Does this allow the verifier to successfully accept a BPF program without
tracking the reference, potentially leading to a kernel resource leak when
the reference isn't released at runtime?


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

Re: [PATCH bpf-next v2 1/5] bpf: Fix dead error check on acquire_reference() in check_kfunc_call

[Amery Hung]

On Fri, Jun 5, 2026 at 12:27 PM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 7d27ba396d32..a741bf447931 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -12817,9 +12817,10 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> >       struct bpf_kfunc_call_arg_meta meta;
> >       struct bpf_insn_aux_data *insn_aux;
> >       int err, insn_idx = *insn_idx_p;
> > -     u32 i, nargs, ptr_type_id, id;
> >       const struct btf_param *args;
> > +     u32 i, nargs, ptr_type_id;
> >       struct btf *desc_btf;
> > +     int id;
> >
> >       /* skip for now, but return error when we find this in fixup_kfunc_call */
> >       if (!insn->imm)
>
> This patch fixes the missing error check in check_kfunc_call(), but
> could a similar issue exist in do_check_common() when initializing
> struct_ops program arguments?
>
> Looking at do_check_common(), there's a similar pattern where
> acquire_reference() is called:
>
> kernel/bpf/verifier.c:do_check_common() {
>     ...
>     for (i = 0; i < aux->ctx_arg_info_size; i++)
>         aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
>                                       acquire_reference(env, 0, 0) : 0;
>     ...
> }
>
> Since acquire_reference() can return a negative error like -ENOMEM,
> assigning it directly to aux->ctx_arg_info[i].ref_id (which is a u32)
> without checking for errors would swallow the error code.
>
> Does this allow the verifier to successfully accept a BPF program without
> tracking the reference, potentially leading to a kernel resource leak when
> the reference isn't released at runtime?

This is fixed in the next patch

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

Re: [PATCH bpf-next v2 2/5] bpf: Check acquire_reference() error for "__ref" struct_ops arguments

[Amery Hung]

On Fri, Jun 5, 2026 at 12:27 PM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index a741bf447931..5a69af569089 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -18363,9 +18363,15 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
> >
> >       /* Acquire references for struct_ops program arguments tagged with "__ref" */
> >       if (!subprog && env->prog->type == BPF_PROG_TYPE_STRUCT_OPS) {
> > -             for (i = 0; i < aux->ctx_arg_info_size; i++)
> > -                     aux->ctx_arg_info[i].ref_id = aux->ctx_arg_info[i].refcounted ?
> > -                                                   acquire_reference(env, 0, 0) : 0;
> > +             for (i = 0; i < aux->ctx_arg_info_size; i++) {
> > +                     int id;
> > +
> > +                     id = aux->ctx_arg_info[i].refcounted ? acquire_reference(env, 0, 0) : 0;
> > +                     if (id < 0)
> > +                             return id;
>
> Does this direct return bypass the cleanup path and leak env->cur_state?
>
> Earlier in do_check_common(), env->cur_state is allocated via kzalloc_obj().
> Other error paths in this function set the return variable and jump to the
> out label to call free_states(env) for proper cleanup.
>
> Should this be updated to set ret = id and goto out instead of returning
> directly?
>

Yes. Will fix it and resend.

> > +
> > +                     aux->ctx_arg_info[i].ref_id = id;
> > +             }
> >       }
> >
> >       ret = do_check(env);
>
>
> ---
> 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/27034008594

Re: [PATCH bpf-next v2 3/5] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR

[Amery Hung]

On Fri, Jun 5, 2026 at 12:27 PM <bot+bpf-ci@kernel.org> wrote:
>
> > diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> > index 5945956a7573..06d9ae24f006 100644
> > --- a/kernel/bpf/states.c
> > +++ b/kernel/bpf/states.c
> > @@ -890,6 +890,9 @@ static bool refsafe(struct bpf_verifier_state *old, struct bpf_verifier_state *c
> >                       return false;
> >               switch (old->refs[i].type) {
> >               case REF_TYPE_PTR:
> > +                     if (!check_ids(old->refs[i].parent_id, cur->refs[i].parent_id, idmap))
> > +                             return false;
> > +                     break;
> >               case REF_TYPE_IRQ:
> >                       break;
> >               case REF_TYPE_LOCK:
>
> This change now feeds parent_id values through check_ids() in addition to
> register and stack slot ids.
>
> In the v1 discussion, Eduard Zingerman noted that since not all ids
> correspond to register or stack slots anymore, the WARN_ON_ONCE(1) in
> check_ids() on idmap exhaustion should be removed:
>
> kernel/bpf/states.c:check_ids() {
>         ...
>         /* Reached the end of known mappings; haven't seen this id before */
>         if (idmap->cnt < BPF_ID_MAP_SIZE) {
>                 map[idmap->cnt].old = old_id;
>                 map[idmap->cnt].cur = cur_id;
>                 idmap->cnt++;
>                 return true;
>         }
>
>         /* We ran out of idmap slots, which should be impossible */
>         WARN_ON_ONCE(1);
>         return false;
> }
>
> The concern raised by the CI and Sashiko bots was that a BPF program
> leaking many references could exhaust BPF_ID_MAP_SIZE and trigger this
> warning, which under panic_on_warn becomes a denial of service. Returning
> false on exhaustion was agreed to be acceptable; only the warning was
> questioned.
>
> The reply on the thread acknowledged this ("Ack. I will drop
> WARN_ON_ONCE(1)."), but the WARN_ON_ONCE(1) is still present in this commit.
>
> Was the WARN_ON_ONCE(1) removal intended for a separate patch in the
> series, or should it be folded in here?

This is dropped in patch 4

>
> Reference:
> https://lore.kernel.org/bpf/6d86c2fae9be63eb110ebd5b3fc19bc4cf1de2dc.camel@gmail.com/
>
>
> ---
> 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/27034008594

Re: [PATCH bpf-next v2 4/5] bpf: Remove WARN_ON_ONCE in check_ids()

[Amery Hung]

On Fri, Jun 5, 2026 at 12:08 PM <bot+bpf-ci@kernel.org> wrote:
>
> > bpf: Remove WARN_ON_ONCE in check_ids()
> >
> > check_ids() warned when it ran out of idmap slots, assuming this was
> > impossible because the slots are bounded by the number of registers and
> > stack slots. That assumption no longer holds: referenced dynptrs acquire
> > an intermediate reference that lives in refs[] but is not backed by any
> > register or stack slot [0], so a program can accumulate more reference
> > ids than the idmap can hold and exhaust it.
> >
> > Exhaustion is fine for verification correctness. check_ids() already
> > returns false, which makes the states compare as not equivalent and
> > prevents unsound pruning. The only effect of the WARN_ON_ONCE() is log
> > noise, or a panic under panic_on_warn. Drop the warning and keep
> > returning false.
> >
> > [0] 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
> >
> > Signed-off-by: Amery Hung <ameryhung@gmail.com>
>
> This removes a WARN_ON_ONCE() that can panic under panic_on_warn, and
> the commit body explains the idmap exhaustion only became reachable once
> referenced dynptrs began acquiring an intermediate reference in refs[]
> that is not backed by a register or stack slot.
>
> Should this carry a Fixes: tag pointing at the commit that introduced
> that behavior, which the body already cites as [0]?

I don't think a fix tag is needed, This is just to reduce noise. Not a bug.

>
>   Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
>
> ---
> 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/27034008594