All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
@ 2026-06-03 20:42 Sechang Lim
  2026-06-03 20:56 ` sashiko-bot
  2026-06-05 16:59 ` Yonghong Song
  0 siblings, 2 replies; 4+ messages in thread
From: Sechang Lim @ 2026-06-03 20:42 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

Bail out when current has no pid namespace.

Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
 kernel/bpf/helpers.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b5314c9fed3c..4646a915bf98 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2913,6 +2913,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
 	struct task_struct *p;
 
 	rcu_read_lock();
+	if (!task_active_pid_ns(current)) {
+		rcu_read_unlock();
+		return NULL;
+	}
 	p = find_task_by_vpid(vpid);
 	if (p)
 		p = bpf_task_acquire(p);
-- 
2.43.0


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

* Re: [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-03 20:42 [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
@ 2026-06-03 20:56 ` sashiko-bot
  2026-06-05 16:59 ` Yonghong Song
  1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-06-03 20:56 UTC (permalink / raw)
  To: Sechang Lim; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch prevents a NULL pointer dereference in `bpf_task_from_vpid()`, but leaves the root vulnerability unpatched in `find_pid_ns()` and `find_task_by_vpid()`, exposing other callers like `trace_user_stack_print()` to the exact same crash.
- [High] `bpf_task_from_vpid()` uses `current` to determine the target PID namespace, which produces arbitrary, non-deterministic results when called from interrupt/network BPF programs.
--

commit 061d0db0a968fb39f30d4aaead03a668556d0e21
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf: fix NULL pointer dereference in bpf_task_from_vpid()
    
bpf_task_from_vpid() looks up a task in the pid namespace of the current
task. This patch adds a check to bail out when current has no pid namespace
to avoid a NULL pointer dereference in find_pid_ns().

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3cf..4646a915bf98c 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2913,6 +2913,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>  	struct task_struct *p;
>  
>  	rcu_read_lock();
> +	if (!task_active_pid_ns(current)) {
> +		rcu_read_unlock();
> +		return NULL;
> +	}

[Severity: High]
This is a pre-existing issue, but while this patch prevents the crash in
bpf_task_from_vpid(), does it leave other callers of find_task_by_vpid()
vulnerable to the exact same NULL pointer dereference?

find_task_by_vpid() evaluates task_active_pid_ns(current) and passes the
result directly to find_pid_ns():

kernel/pid.c:find_pid_ns() {
        return idr_find(&ns->idr, nr);
}

If current has cleared its thread_pid during do_exit(), the namespace
pointer is NULL, and find_pid_ns() unconditionally dereferences it.

Other callers like trace_user_stack_print() in kernel/trace/trace_output.c
can still trigger this crash. For example, if the kernel oopses inside
do_exit() after detach_pid(), ftrace_dump_on_oops() can be invoked in a
panic context. If the trace buffer contains a user stack entry,
trace_user_stack_print() calls find_task_by_vpid(), potentially triggering
a recursive double-fault that prevents the panic log from flushing.

Would it be safer to add the NULL check directly inside find_pid_ns()
or find_task_by_vpid() instead?

[Severity: High]
This isn't a bug introduced by this patch, but does the use of current here
lead to arbitrary results for BPF programs running in interrupt context?

This kfunc is available for program types like cgroup_skb and xdp, which
can run in softirq or hardware interrupt context. In these contexts, current
points to whatever random task happened to be interrupted by the packet.

Since bpf_task_from_vpid() looks up the virtual PID relative to
task_active_pid_ns(current), won't passing the same vpid for the same
packet look up the PID in a completely random container or root namespace?

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

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

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

* Re: [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-03 20:42 [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
  2026-06-03 20:56 ` sashiko-bot
@ 2026-06-05 16:59 ` Yonghong Song
  2026-06-05 19:00   ` Sechang Lim
  1 sibling, 1 reply; 4+ messages in thread
From: Yonghong Song @ 2026-06-05 16:59 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/3/26 1:42 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
>
> Bail out when current has no pid namespace.
>
> Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
> ---
>   kernel/bpf/helpers.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3c..4646a915bf98 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2913,6 +2913,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>   	struct task_struct *p;
>   
>   	rcu_read_lock();
> +	if (!task_active_pid_ns(current)) {
> +		rcu_read_unlock();
> +		return NULL;
> +	}

In softirq context, I think we should return NULL for this kfunc.
Your above fix solves crash problem. But even not crash, the
below task 'p' may not be user expected since the 'current' (and
its namespace) is random.

Maybe we can do:

+	if (in_interrupt())
+		return NULL;
+
	rcu_read_lock();
	...

In bpf selftest task_kfunc_success.c, all programs are 'syscall'
programs which garantees process context.

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


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

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

On Fri, Jun 05, 2026 at 09:59:49AM -0700, Yonghong Song wrote:
>In softirq context, I think we should return NULL for this kfunc.
>Your above fix solves crash problem. But even not crash, the
>below task 'p' may not be user expected since the 'current' (and
>its namespace) is random.
>
>Maybe we can do:
>
>+	if (in_interrupt())
>+		return NULL;
>+
>	rcu_read_lock();
>	...
>

Agreed. I'll send a v2 using this approach.

Thanks for the review.

Sechang

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

end of thread, other threads:[~2026-06-05 19:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-03 20:42 [PATCH bpf] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
2026-06-03 20:56 ` sashiko-bot
2026-06-05 16:59 ` Yonghong Song
2026-06-05 19:00   ` Sechang Lim

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.