All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
@ 2026-06-05 20:04 Sechang Lim
  2026-06-05 20:14 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sechang Lim @ 2026-06-05 20:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Juntong Deng, bpf, linux-kernel

bpf_task_from_vpid() looks up a task in the pid namespace of the
current task, via find_task_by_vpid():

  find_task_by_vpid(vpid)
    find_task_by_pid_ns(vpid, task_active_pid_ns(current))
      find_pid_ns(nr, ns) -> idr_find(&ns->idr, nr)

cgroup_skb programs run in softirq, which may interrupt a task that is
itself in do_exit(). Once that task has passed
exit_notify() -> release_task() -> __unhash_process(), its thread_pid is
cleared, so task_active_pid_ns(current) returns NULL and find_pid_ns()
dereferences &NULL->idr:

  BUG: kernel NULL pointer dereference, address: 0000000000000050
  RIP: 0010:idr_find+0x11/0x30 lib/idr.c:176
  Call Trace:
   <IRQ>
   find_pid_ns kernel/pid.c:370 [inline]
   find_task_by_pid_ns+0x3b/0xe0 kernel/pid.c:485
   bpf_task_from_vpid+0x5b/0x200 kernel/bpf/helpers.c:2916
   bpf_prog_run_array_cg+0x17e/0x530 kernel/bpf/cgroup.c:81
   __cgroup_bpf_run_filter_skb+0x12b/0x250 kernel/bpf/cgroup.c:1612
   sk_filter_trim_cap+0x1dc/0x4c0 net/core/filter.c:148
   tcp_v4_rcv+0x18d1/0x2200 net/ipv4/tcp_ipv4.c:2223
   </IRQ>
   <TASK>
   do_exit+0xa63/0x1270 kernel/exit.c:1010
   get_signal+0x141c/0x1530 kernel/signal.c:3037

Return NULL when called from interrupt context.

Suggested-by: Yonghong Song <yonghong.song@linux.dev> 
Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
v2:
 - Reject calls from interrupt context (Yonghong Song)

v1:
 - https://lore.kernel.org/bpf/20260603204206.773482-1-rhkrqnwk98@gmail.com/

 kernel/bpf/helpers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b5314c9fed3c..890202361b53 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2912,6 +2912,9 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
 {
 	struct task_struct *p;
 
+	if (in_interrupt())
+		return NULL;
+
 	rcu_read_lock();
 	p = find_task_by_vpid(vpid);
 	if (p)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-05 20:04 [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
@ 2026-06-05 20:14 ` sashiko-bot
  2026-06-05 20:32 ` bot+bpf-ci
  2026-06-06  3:13 ` Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-05 20:14 UTC (permalink / raw)
  To: Sechang Lim; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete fix and functional regression using deprecated API in `bpf_task_from_vpid()`
--

commit 0e07d8082fa185913ccf64e5ced29e9a7fa63669
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf: fix NULL pointer dereference in bpf_task_from_vpid()

This commit attempts to fix a NULL pointer dereference in
bpf_task_from_vpid() when the current task is in the late stages of
do_exit(). It adds an in_interrupt() check to return NULL before calling
find_task_by_vpid() in order to prevent crashes from cgroup_skb programs.

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3cf..890202361b533 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2912,6 +2912,9 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>  {
>  	struct task_struct *p;
>  
> +	if (in_interrupt())
> +		return NULL;

[Severity: High]
Does this check fully prevent the crash during task exit?

While this prevents the crash when an interrupt preempts an exiting task,
it appears it might not protect synchronous BPF tracing programs (like
kprobes or sched_switch tracepoints) executing in task context during
do_exit(). 

Once __unhash_process() has cleared thread_pid, in_interrupt() is false
in task context, but the namespace is still NULL. Wouldn't the crash 
still occur in find_task_by_vpid()?

kernel/pid.c:find_task_by_vpid() {
	return find_task_by_pid_ns(vnr, task_active_pid_ns(current));
}

Additionally, in_interrupt() is known to return true in normal task context
whenever bottom-halves are disabled, such as inside a spin_lock_bh() region.

Will this cause bpf_task_from_vpid() to unexpectedly fail for valid
tracing programs attached to living tasks that just happen to be holding
a BH-disabled lock? 

Could we verify the namespace directly instead to avoid these issues,
perhaps by checking if task_active_pid_ns(current) is NULL?

> +
>  	rcu_read_lock();
>  	p = find_task_by_vpid(vpid);
>  	if (p)

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-05 20:04 [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
  2026-06-05 20:14 ` sashiko-bot
@ 2026-06-05 20:32 ` bot+bpf-ci
  2026-06-06  3:13 ` Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: bot+bpf-ci @ 2026-06-05 20:32 UTC (permalink / raw)
  To: rhkrqnwk98, ast, daniel, andrii, eddyz87, memxor
  Cc: martin.lau, song, yonghong.song, jolsa, juntong.deng, bpf,
	linux-kernel, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai

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

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3c..890202361b53 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2912,6 +2912,9 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>  {
>  	struct task_struct *p;
>
> +	if (in_interrupt())
> +		return NULL;
> +

Does this check fully prevent the crash during task exit?

While this prevents the crash when an interrupt preempts an exiting task, it
appears it might not protect synchronous BPF tracing programs (like kprobes or
sched_switch tracepoints) executing in task context during do_exit().

Once __unhash_process() has cleared thread_pid, in_interrupt() returns false
in task context, but task_active_pid_ns(current) is still NULL. Wouldn't the
crash still occur in find_task_by_vpid() -> find_task_by_pid_ns() ->
find_pid_ns() -> idr_find(&NULL->idr)?

Additionally, in_interrupt() returns true in normal task context whenever
bottom-halves are disabled, such as inside a spin_lock_bh() region. Will this
cause bpf_task_from_vpid() to unexpectedly fail for valid tracing programs
attached to living tasks that happen to be holding a BH-disabled lock?

Could we verify the namespace directly instead to avoid these issues, perhaps
by checking if task_active_pid_ns(current) is NULL?

>  	rcu_read_lock();
>  	p = find_task_by_vpid(vpid);
>  	if (p)


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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-05 20:04 [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
  2026-06-05 20:14 ` sashiko-bot
  2026-06-05 20:32 ` bot+bpf-ci
@ 2026-06-06  3:13 ` Yonghong Song
  2 siblings, 0 replies; 4+ messages in thread
From: Yonghong Song @ 2026-06-06  3:13 UTC (permalink / raw)
  To: Sechang Lim, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi
  Cc: Martin KaFai Lau, Song Liu, Jiri Olsa, Juntong Deng, bpf,
	linux-kernel



On 6/5/26 1:04 PM, Sechang Lim wrote:
> bpf_task_from_vpid() looks up a task in the pid namespace of the
> current task, via find_task_by_vpid():
>
>    find_task_by_vpid(vpid)
>      find_task_by_pid_ns(vpid, task_active_pid_ns(current))
>        find_pid_ns(nr, ns) -> idr_find(&ns->idr, nr)
>
> cgroup_skb programs run in softirq, which may interrupt a task that is
> itself in do_exit(). Once that task has passed
> exit_notify() -> release_task() -> __unhash_process(), its thread_pid is
> cleared, so task_active_pid_ns(current) returns NULL and find_pid_ns()
> dereferences &NULL->idr:
>
>    BUG: kernel NULL pointer dereference, address: 0000000000000050
>    RIP: 0010:idr_find+0x11/0x30 lib/idr.c:176
>    Call Trace:
>     <IRQ>
>     find_pid_ns kernel/pid.c:370 [inline]
>     find_task_by_pid_ns+0x3b/0xe0 kernel/pid.c:485
>     bpf_task_from_vpid+0x5b/0x200 kernel/bpf/helpers.c:2916
>     bpf_prog_run_array_cg+0x17e/0x530 kernel/bpf/cgroup.c:81
>     __cgroup_bpf_run_filter_skb+0x12b/0x250 kernel/bpf/cgroup.c:1612
>     sk_filter_trim_cap+0x1dc/0x4c0 net/core/filter.c:148
>     tcp_v4_rcv+0x18d1/0x2200 net/ipv4/tcp_ipv4.c:2223
>     </IRQ>
>     <TASK>
>     do_exit+0xa63/0x1270 kernel/exit.c:1010
>     get_signal+0x141c/0x1530 kernel/signal.c:3037
>
> Return NULL when called from interrupt context.
>
> Suggested-by: Yonghong Song <yonghong.song@linux.dev>
> Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
> ---
> v2:
>   - Reject calls from interrupt context (Yonghong Song)
>
> v1:
>   - https://lore.kernel.org/bpf/20260603204206.773482-1-rhkrqnwk98@gmail.com/
>
>   kernel/bpf/helpers.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3c..890202361b53 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2912,6 +2912,9 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>   {
>   	struct task_struct *p;
>   
> +	if (in_interrupt())
> +		return NULL;
> +

AI suggested we should have

  	rcu_read_lock();
+ if (!task_active_pid_ns(current)) { + rcu_read_unlock(); + return 
NULL; + }  	p = find_task_by_vpid(vpid);

too. I was aware of this in v1 and throught it is very unlikely
as the tracing prog will need to hook into a few functions
after current->thread_pid to be NULL and we may skip it.

But the unlikely does not mean it won't happen, so let us
just add your change in v1 as well. With this,

Acked-by: Yonghong Song <yonghong.song@linux.dev>

>   	rcu_read_lock();
>   	p = find_task_by_vpid(vpid);
>   	if (p)


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-06-06  3:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-05 20:04 [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
2026-06-05 20:14 ` sashiko-bot
2026-06-05 20:32 ` bot+bpf-ci
2026-06-06  3:13 ` Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.