* [PATCH bpf-next v1 0/3] Object relationship refactor followup
@ 2026-06-04 22:09 Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Amery Hung @ 2026-06-04 22:09 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team
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 (3):
bpf: Fix dead error check on acquire_reference() in check_kfunc_call
bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test
kernel/bpf/states.c | 3 +++
kernel/bpf/verifier.c | 3 ++-
tools/testing/selftests/bpf/progs/file_reader_fail.c | 8 ++++----
3 files changed, 9 insertions(+), 5 deletions(-)
--
2.53.0-Meta
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
@ 2026-06-04 22:09 ` Amery Hung
2026-06-04 22:43 ` bot+bpf-ci
2026-06-04 23:16 ` Eduard Zingerman
2026-06-04 22:09 ` [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR Amery Hung
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Amery Hung @ 2026-06-04 22:09 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team
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.
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
@ 2026-06-04 22:09 ` Amery Hung
2026-06-04 22:59 ` bot+bpf-ci
2026-06-04 22:09 ` [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test Amery Hung
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Amery Hung @ 2026-06-04 22:09 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team
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.
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR Amery Hung
@ 2026-06-04 22:09 ` Amery Hung
2026-06-04 23:22 ` Eduard Zingerman
2026-06-04 22:14 ` [PATCH bpf-next v1 0/3] Object relationship refactor followup Kumar Kartikeya Dwivedi
2026-06-05 21:20 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 16+ messages in thread
From: Amery Hung @ 2026-06-04 22:09 UTC (permalink / raw)
To: bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 0/3] Object relationship refactor followup
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
` (2 preceding siblings ...)
2026-06-04 22:09 ` [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test Amery Hung
@ 2026-06-04 22:14 ` Kumar Kartikeya Dwivedi
2026-06-04 23:20 ` Amery Hung
2026-06-05 21:20 ` patchwork-bot+netdevbpf
4 siblings, 1 reply; 16+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-06-04 22:14 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, eddyz87,
martin.lau, mykyta.yatsenko5, kernel-team
On Fri, 5 Jun 2026 at 00:09, Amery Hung <ameryhung@gmail.com> wrote:
>
> 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/
>
>
bpf-next makes sense, but Fixes: tag looks missing?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
@ 2026-06-04 22:43 ` bot+bpf-ci
2026-06-04 23:16 ` Eduard Zingerman
1 sibling, 0 replies; 16+ messages in thread
From: bot+bpf-ci @ 2026-06-04 22:43 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team, ast, andrii,
daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 998 bytes --]
> bpf: Fix dead error check on acquire_reference() in check_kfunc_call
>
> 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.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
This looks like a bug fix, but it does not carry a Fixes: tag. The dead
error check appears to have been introduced when check_kfunc_call() began
using the function-scoped 'u32 i, nargs, ptr_type_id, id;' declaration in
place of the earlier block-local 'int id'.
Should this include:
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/26983135146
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-04 22:09 ` [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR Amery Hung
@ 2026-06-04 22:59 ` bot+bpf-ci
2026-06-04 23:21 ` Eduard Zingerman
0 siblings, 1 reply; 16+ messages in thread
From: bot+bpf-ci @ 2026-06-04 22:59 UTC (permalink / raw)
To: ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, ameryhung, kernel-team, ast, andrii,
daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 3078 bytes --]
> commit 89f2be9817e074d02702d22068104fb9fd9aa103
> Author: Amery Hung <ameryhung@gmail.com>
> Date: Thu Jun 4 15:09:03 2026 -0700
>
> bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
>
> 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.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
This looks like a bug fix for parent_id tracking support. Should this
include:
Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
When commit 308c7a0ae885 added parent_id tracking, it updated several
safety-checking functions to compare parent_id but missed adding the
parent_id check in refsafe() for the REF_TYPE_PTR case. This caused states
with different parent objects to be incorrectly treated as equivalent
during state pruning.
> 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 adds an extra check_ids() call for parent_id. Could this make it
easier to trigger the WARN_ON_ONCE() in check_ids() via BPF_ID_MAP_SIZE
exhaustion?
Since acquired_refs has no explicit limit, a BPF program could acquire and
leak many references (e.g., via bpf_sk_lookup_tcp) before a branch merge
point. When states_equal() invokes refsafe(), it calls check_ids() for
each acquired reference.
The idmap has a fixed size based only on registers and stack slots:
include/linux/bpf_verifier.h:
#define BPF_ID_MAP_SIZE ((MAX_BPF_REG + MAX_BPF_STACK / BPF_REG_SIZE + \
MAX_STACK_ARG_SLOTS) * MAX_CALL_FRAMES)
If a program accumulates enough leaked references, checking them all will
run out of idmap slots and trigger the warning:
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) {
...
return true;
}
/* We ran out of idmap slots, which should be impossible */
WARN_ON_ONCE(1);
return false;
}
With this patch now checking both reference id and parent_id for
REF_TYPE_PTR (doubling the idmap slot usage per reference), does this
allow exhausting BPF_ID_MAP_SIZE much faster?
Could a user intentionally leak enough references to trigger this warning
and potentially panic the kernel if panic_on_warn is set?
---
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/26983135146
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
2026-06-04 22:43 ` bot+bpf-ci
@ 2026-06-04 23:16 ` Eduard Zingerman
1 sibling, 0 replies; 16+ messages in thread
From: Eduard Zingerman @ 2026-06-04 23:16 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, memxor, martin.lau,
mykyta.yatsenko5, kernel-team
On Thu, 2026-06-04 at 15:09 -0700, Amery Hung wrote:
> 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.
>
> Signed-off-by: Amery Hung <ameryhung@gmail.com>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 0/3] Object relationship refactor followup
2026-06-04 22:14 ` [PATCH bpf-next v1 0/3] Object relationship refactor followup Kumar Kartikeya Dwivedi
@ 2026-06-04 23:20 ` Amery Hung
0 siblings, 0 replies; 16+ messages in thread
From: Amery Hung @ 2026-06-04 23:20 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Amery Hung, bpf, netdev, alexei.starovoitov, andrii, daniel,
eddyz87, martin.lau, mykyta.yatsenko5, kernel-team
On Thu, Jun 4, 2026 at 3:15 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
>
> On Fri, 5 Jun 2026 at 00:09, Amery Hung <ameryhung@gmail.com> wrote:
> >
> > 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/
> >
> >
>
> bpf-next makes sense, but Fixes: tag looks missing?
I will add fix tags to both patch 1 and 2 and respin.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-04 22:59 ` bot+bpf-ci
@ 2026-06-04 23:21 ` Eduard Zingerman
2026-06-04 23:27 ` Amery Hung
0 siblings, 1 reply; 16+ messages in thread
From: Eduard Zingerman @ 2026-06-04 23:21 UTC (permalink / raw)
To: bot+bpf-ci, ameryhung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, memxor, martin.lau,
mykyta.yatsenko5, kernel-team, ast, yonghong.song, clm,
ihor.solodrai
On Thu, 2026-06-04 at 22:59 +0000, bot+bpf-ci@kernel.org wrote:
> > commit 89f2be9817e074d02702d22068104fb9fd9aa103
> > Author: Amery Hung <ameryhung@gmail.com>
> > Date: Thu Jun 4 15:09:03 2026 -0700
> >
> > bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
> >
> > 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.
> >
> > Signed-off-by: Amery Hung <ameryhung@gmail.com>
>
> This looks like a bug fix for parent_id tracking support. Should this
> include:
>
> Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
>
> When commit 308c7a0ae885 added parent_id tracking, it updated several
> safety-checking functions to compare parent_id but missed adding the
> parent_id check in refsafe() for the REF_TYPE_PTR case. This caused states
> with different parent objects to be incorrectly treated as equivalent
> during state pruning.
>
> > 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 adds an extra check_ids() call for parent_id. Could this make it
> easier to trigger the WARN_ON_ONCE() in check_ids() via BPF_ID_MAP_SIZE
> exhaustion?
I think the bot has the point here. Let's switch to reallocating idmap
or failing check_ids() if there are to many ids (probably an
unrealistic scenario).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test
2026-06-04 22:09 ` [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test Amery Hung
@ 2026-06-04 23:22 ` Eduard Zingerman
0 siblings, 0 replies; 16+ messages in thread
From: Eduard Zingerman @ 2026-06-04 23:22 UTC (permalink / raw)
To: Amery Hung, bpf
Cc: netdev, alexei.starovoitov, andrii, daniel, memxor, martin.lau,
mykyta.yatsenko5, kernel-team
On Thu, 2026-06-04 at 15:09 -0700, Amery Hung wrote:
> 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>
> ---
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
[...]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-04 23:21 ` Eduard Zingerman
@ 2026-06-04 23:27 ` Amery Hung
2026-06-05 17:57 ` Amery Hung
0 siblings, 1 reply; 16+ messages in thread
From: Amery Hung @ 2026-06-04 23:27 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bot+bpf-ci, bpf, netdev, alexei.starovoitov, andrii, daniel,
memxor, martin.lau, mykyta.yatsenko5, kernel-team, ast,
yonghong.song, clm, ihor.solodrai
On Thu, Jun 4, 2026 at 4:21 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2026-06-04 at 22:59 +0000, bot+bpf-ci@kernel.org wrote:
> > > commit 89f2be9817e074d02702d22068104fb9fd9aa103
> > > Author: Amery Hung <ameryhung@gmail.com>
> > > Date: Thu Jun 4 15:09:03 2026 -0700
> > >
> > > bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
> > >
> > > 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.
> > >
> > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> >
> > This looks like a bug fix for parent_id tracking support. Should this
> > include:
> >
> > Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
> >
> > When commit 308c7a0ae885 added parent_id tracking, it updated several
> > safety-checking functions to compare parent_id but missed adding the
> > parent_id check in refsafe() for the REF_TYPE_PTR case. This caused states
> > with different parent objects to be incorrectly treated as equivalent
> > during state pruning.
> >
> > > 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 adds an extra check_ids() call for parent_id. Could this make it
> > easier to trigger the WARN_ON_ONCE() in check_ids() via BPF_ID_MAP_SIZE
> > exhaustion?
>
> I think the bot has the point here. Let's switch to reallocating idmap
> or failing check_ids() if there are to many ids (probably an
> unrealistic scenario).
Make sense. I will switch to failing check_ids() first since it is
unlikely for sane programs.
I will also change bpf_insn_access_aux::ref_id to int as pointed out by sashiko.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-04 23:27 ` Amery Hung
@ 2026-06-05 17:57 ` Amery Hung
2026-06-05 18:04 ` Eduard Zingerman
0 siblings, 1 reply; 16+ messages in thread
From: Amery Hung @ 2026-06-05 17:57 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bot+bpf-ci, bpf, netdev, alexei.starovoitov, andrii, daniel,
memxor, martin.lau, mykyta.yatsenko5, kernel-team, ast,
yonghong.song, clm, ihor.solodrai
On Thu, Jun 4, 2026 at 4:27 PM Amery Hung <ameryhung@gmail.com> wrote:
>
> On Thu, Jun 4, 2026 at 4:21 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Thu, 2026-06-04 at 22:59 +0000, bot+bpf-ci@kernel.org wrote:
> > > > commit 89f2be9817e074d02702d22068104fb9fd9aa103
> > > > Author: Amery Hung <ameryhung@gmail.com>
> > > > Date: Thu Jun 4 15:09:03 2026 -0700
> > > >
> > > > bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
> > > >
> > > > 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.
> > > >
> > > > Signed-off-by: Amery Hung <ameryhung@gmail.com>
> > >
> > > This looks like a bug fix for parent_id tracking support. Should this
> > > include:
> > >
> > > Fixes: 308c7a0ae885 ("bpf: Refactor object relationship tracking and fix dynptr UAF bug")
> > >
> > > When commit 308c7a0ae885 added parent_id tracking, it updated several
> > > safety-checking functions to compare parent_id but missed adding the
> > > parent_id check in refsafe() for the REF_TYPE_PTR case. This caused states
> > > with different parent objects to be incorrectly treated as equivalent
> > > during state pruning.
> > >
> > > > 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 adds an extra check_ids() call for parent_id. Could this make it
> > > easier to trigger the WARN_ON_ONCE() in check_ids() via BPF_ID_MAP_SIZE
> > > exhaustion?
> >
> > I think the bot has the point here. Let's switch to reallocating idmap
> > or failing check_ids() if there are to many ids (probably an
> > unrealistic scenario).
>
> Make sense. I will switch to failing check_ids() first since it is
> unlikely for sane programs.
>
Actually, looking at it a bit more. I think I would hold on changing
check_ids().
This patch should not increase idmap usage. parent_id refers to id and
therefore check_ids(parent_id) will reuse an existing slot.
The id_map is indeed more likely to be triggered since there is also
an intermediate reference for reference dynptr introduce during the
refactor, but that should still be very unlikely under normal usage.
check_ids() also already fails in this scenario by returning false. So
overall, I am not sure if we need to change it.
WDYT?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-05 17:57 ` Amery Hung
@ 2026-06-05 18:04 ` Eduard Zingerman
2026-06-05 18:09 ` Amery Hung
0 siblings, 1 reply; 16+ messages in thread
From: Eduard Zingerman @ 2026-06-05 18:04 UTC (permalink / raw)
To: Amery Hung
Cc: bot+bpf-ci, bpf, netdev, alexei.starovoitov, andrii, daniel,
memxor, martin.lau, mykyta.yatsenko5, kernel-team, ast,
yonghong.song, clm, ihor.solodrai
On Fri, 2026-06-05 at 10:57 -0700, Amery Hung wrote:
[...]
> Actually, looking at it a bit more. I think I would hold on changing
> check_ids().
>
> This patch should not increase idmap usage. parent_id refers to id and
> therefore check_ids(parent_id) will reuse an existing slot.
>
> The id_map is indeed more likely to be triggered since there is also
> an intermediate reference for reference dynptr introduce during the
> refactor, but that should still be very unlikely under normal usage.
> check_ids() also already fails in this scenario by returning false. So
> overall, I am not sure if we need to change it.
>
> WDYT?
I agree that it should be fine to just return false from check_ids()
if there is no space in the idmap. However, since not all ids
correspond to register/stack slots anymore, I think that
WARN_ON_ONCE(1) should be removed.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
2026-06-05 18:04 ` Eduard Zingerman
@ 2026-06-05 18:09 ` Amery Hung
0 siblings, 0 replies; 16+ messages in thread
From: Amery Hung @ 2026-06-05 18:09 UTC (permalink / raw)
To: Eduard Zingerman
Cc: bot+bpf-ci, bpf, netdev, alexei.starovoitov, andrii, daniel,
memxor, martin.lau, mykyta.yatsenko5, kernel-team, ast,
yonghong.song, clm, ihor.solodrai
On Fri, Jun 5, 2026 at 11:04 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Fri, 2026-06-05 at 10:57 -0700, Amery Hung wrote:
>
> [...]
>
> > Actually, looking at it a bit more. I think I would hold on changing
> > check_ids().
> >
> > This patch should not increase idmap usage. parent_id refers to id and
> > therefore check_ids(parent_id) will reuse an existing slot.
> >
> > The id_map is indeed more likely to be triggered since there is also
> > an intermediate reference for reference dynptr introduce during the
> > refactor, but that should still be very unlikely under normal usage.
> > check_ids() also already fails in this scenario by returning false. So
> > overall, I am not sure if we need to change it.
> >
> > WDYT?
>
> I agree that it should be fine to just return false from check_ids()
> if there is no space in the idmap. However, since not all ids
> correspond to register/stack slots anymore, I think that
> WARN_ON_ONCE(1) should be removed.
Ack. I will drop WARN_ON_ONCE(1).
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH bpf-next v1 0/3] Object relationship refactor followup
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
` (3 preceding siblings ...)
2026-06-04 22:14 ` [PATCH bpf-next v1 0/3] Object relationship refactor followup Kumar Kartikeya Dwivedi
@ 2026-06-05 21:20 ` patchwork-bot+netdevbpf
4 siblings, 0 replies; 16+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-06-05 21:20 UTC (permalink / raw)
To: Amery Hung
Cc: bpf, netdev, alexei.starovoitov, andrii, daniel, eddyz87, memxor,
martin.lau, mykyta.yatsenko5, kernel-team
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Thu, 4 Jun 2026 15:09:01 -0700 you wrote:
> 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/
>
> [...]
Here is the summary with links:
- [bpf-next,v1,1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call
https://git.kernel.org/bpf/bpf-next/c/a3863aa4f55e
- [bpf-next,v1,2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR
https://git.kernel.org/bpf/bpf-next/c/41025f441fe6
- [bpf-next,v1,3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test
https://git.kernel.org/bpf/bpf-next/c/d83d4f63cb8f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-06-05 21:20 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04 22:09 [PATCH bpf-next v1 0/3] Object relationship refactor followup Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 1/3] bpf: Fix dead error check on acquire_reference() in check_kfunc_call Amery Hung
2026-06-04 22:43 ` bot+bpf-ci
2026-06-04 23:16 ` Eduard Zingerman
2026-06-04 22:09 ` [PATCH bpf-next v1 2/3] bpf: Compare parent_id in refsafe() for REF_TYPE_PTR Amery Hung
2026-06-04 22:59 ` bot+bpf-ci
2026-06-04 23:21 ` Eduard Zingerman
2026-06-04 23:27 ` Amery Hung
2026-06-05 17:57 ` Amery Hung
2026-06-05 18:04 ` Eduard Zingerman
2026-06-05 18:09 ` Amery Hung
2026-06-04 22:09 ` [PATCH bpf-next v1 3/3] selftests/bpf: Use bpf_dynptr_slice() to read file dynptr in leak test Amery Hung
2026-06-04 23:22 ` Eduard Zingerman
2026-06-04 22:14 ` [PATCH bpf-next v1 0/3] Object relationship refactor followup Kumar Kartikeya Dwivedi
2026-06-04 23:20 ` Amery Hung
2026-06-05 21:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox