* [PATCH bpf 0/1] Follow-up fix for potential error pointer dereference in propagate_to_outer_instance()
@ 2025-10-16 10:13 Shardul Bankar
2025-10-16 10:13 ` [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() " Shardul Bankar
0 siblings, 1 reply; 4+ messages in thread
From: Shardul Bankar @ 2025-10-16 10:13 UTC (permalink / raw)
To: bpf
Cc: shardulsb08, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
open list
Hi BPF maintainers,
This patch follows up on my previous submission:
[PATCH v2 bpf] bpf: Fix memory leak in __lookup_instance error path
Link: https://patchwork.kernel.org/project/netdevbpf/list/?series=1012189&state=*
During the review and CI discussions for that patch, a potential issue was
identified in propagate_to_outer_instance(), where get_outer_instance() may
return an ERR_PTR (e.g. -ENOMEM) that is not currently checked before use.
This patch adds a simple IS_ERR() guard and returns the error code to prevent
dereferencing the error pointer.
Thanks,
Shardul
---
Shardul Bankar (1):
bpf: liveness: Handle ERR_PTR from get_outer_instance() in
propagate_to_outer_instance()
kernel/bpf/liveness.c | 2 ++
1 file changed, 2 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() in propagate_to_outer_instance()
2025-10-16 10:13 [PATCH bpf 0/1] Follow-up fix for potential error pointer dereference in propagate_to_outer_instance() Shardul Bankar
@ 2025-10-16 10:13 ` Shardul Bankar
2025-10-16 17:43 ` Eduard Zingerman
0 siblings, 1 reply; 4+ messages in thread
From: Shardul Bankar @ 2025-10-16 10:13 UTC (permalink / raw)
To: bpf
Cc: shardulsb08, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
open list
propagate_to_outer_instance() calls get_outer_instance() and then uses the
returned pointer to reset/commit stack write marks. When get_outer_instance()
fails (e.g., __lookup_instance() returns -ENOMEM), it may return an ERR_PTR.
Without a check, the code dereferences this error pointer.
Protect the call with IS_ERR() and propagate the error.
Fixes: b3698c356ad9 ("bpf: callchain sensitive stack liveness tracking
using CFG")
Reported-by: kernel-patches-review-bot (https://github.com/kernel-patches/bpf/pull/10006#issuecomment-3409419240)
Signed-off-by: Shardul Bankar <shardulsb08@gmail.com>
---
kernel/bpf/liveness.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
index 3c611aba7f52..ae31f9ee4994 100644
--- a/kernel/bpf/liveness.c
+++ b/kernel/bpf/liveness.c
@@ -522,6 +522,8 @@ static int propagate_to_outer_instance(struct bpf_verifier_env *env,
this_subprog_start = callchain_subprog_start(callchain);
outer_instance = get_outer_instance(env, instance);
+ if (IS_ERR(outer_instance))
+ return PTR_ERR(outer_instance);
callsite = callchain->callsites[callchain->curframe - 1];
reset_stack_write_marks(env, outer_instance, callsite);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() in propagate_to_outer_instance()
2025-10-16 10:13 ` [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() " Shardul Bankar
@ 2025-10-16 17:43 ` Eduard Zingerman
2025-10-20 6:13 ` Shardul Bankar
0 siblings, 1 reply; 4+ messages in thread
From: Eduard Zingerman @ 2025-10-16 17:43 UTC (permalink / raw)
To: Shardul Bankar, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, open list
On Thu, 2025-10-16 at 15:43 +0530, Shardul Bankar wrote:
> propagate_to_outer_instance() calls get_outer_instance() and then uses the
> returned pointer to reset/commit stack write marks. When get_outer_instance()
> fails (e.g., __lookup_instance() returns -ENOMEM), it may return an ERR_PTR.
> Without a check, the code dereferences this error pointer.
>
> Protect the call with IS_ERR() and propagate the error.
>
> Fixes: b3698c356ad9 ("bpf: callchain sensitive stack liveness tracking
> using CFG")
> Reported-by: kernel-patches-review-bot (https://github.com/kernel-patches/bpf/pull/10006#issuecomment-3409419240)
> Signed-off-by: Shardul Bankar <shardulsb08@gmail.com>
> ---
This was brought up already in [1]. This is not a bug as check before
propagate_to_outer_instance() call in update_instance() guarantees
that outer instance exists.
We can land this change to avoid confusion, but the fixes tag is
unnecessary.
[1] https://lore.kernel.org/bpf/8430f47f73d8d55a698e85341ece81955355c1fd.camel@gmail.com/
> kernel/bpf/liveness.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index 3c611aba7f52..ae31f9ee4994 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
> @@ -522,6 +522,8 @@ static int propagate_to_outer_instance(struct bpf_verifier_env *env,
>
> this_subprog_start = callchain_subprog_start(callchain);
> outer_instance = get_outer_instance(env, instance);
> + if (IS_ERR(outer_instance))
> + return PTR_ERR(outer_instance);
> callsite = callchain->callsites[callchain->curframe - 1];
>
> reset_stack_write_marks(env, outer_instance, callsite);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() in propagate_to_outer_instance()
2025-10-16 17:43 ` Eduard Zingerman
@ 2025-10-20 6:13 ` Shardul Bankar
0 siblings, 0 replies; 4+ messages in thread
From: Shardul Bankar @ 2025-10-20 6:13 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, open list
On Thu, 2025-10-16 at 10:43 -0700, Eduard Zingerman wrote:
> We can land this change to avoid confusion, but the fixes tag is
> unnecessary.
>
> [1]
> https://lore.kernel.org/bpf/8430f47f73d8d55a698e85341ece81955355c1fd.camel@gmail.com/
>
Thanks, Eduard, for the clarification and reference.
That makes sense — since update_instance() guarantees the outer
instance’s existence, this case wouldn’t be hit in normal execution.
I agree, the Fixes: tag can be dropped. The check mainly serves to make
the intent explicit and avoid future confusion.
I’ve resent the patch without the Fixes: tag.
Link:
https://lore.kernel.org/all/20251020060712.4155702-1-shardulsb08@gmail.com/
Thanks again,
Shardul
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-10-20 6:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 10:13 [PATCH bpf 0/1] Follow-up fix for potential error pointer dereference in propagate_to_outer_instance() Shardul Bankar
2025-10-16 10:13 ` [PATCH bpf 1/1] bpf: liveness: Handle ERR_PTR from get_outer_instance() " Shardul Bankar
2025-10-16 17:43 ` Eduard Zingerman
2025-10-20 6:13 ` Shardul Bankar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox