All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
@ 2026-06-06  9:19 Sechang Lim
  2026-06-06  9:31 ` sashiko-bot
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Sechang Lim @ 2026-06-06  9:19 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 bpf_task_from_vpid() runs in interrupt
context, or when current has no pid namespace.

Acked-by: Yonghong Song <yonghong.song@linux.dev>
Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
---
v3:
 - Also handle current with no pid namespace 

v2:
 - Reject calls from interrupt context (Yonghong Song)
 - https://lore.kernel.org/bpf/20260605200501.1619406-1-rhkrqnwk98@gmail.com/

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

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

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b5314c9fed3c..226c31ccb5d6 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2912,7 +2912,14 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
 {
 	struct task_struct *p;
 
+	if (in_interrupt())
+		return NULL;
+
 	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] 7+ messages in thread

* Re: [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-06  9:19 [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
@ 2026-06-06  9:31 ` sashiko-bot
  2026-06-06  9:58 ` bot+bpf-ci
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-06-06  9:31 UTC (permalink / raw)
  To: Sechang Lim; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The use of the deprecated `in_interrupt()` macro causes `bpf_task_from_vpid()` to incorrectly fail and return `NULL` in valid task contexts when Bottom Halves (BH) are disabled.
--

commit 17040face7147d85a8cf3090be3539991c26df30
Author: Sechang Lim <rhkrqnwk98@gmail.com>

bpf: fix NULL pointer dereference in bpf_task_from_vpid()

This commit addresses a NULL pointer dereference in bpf_task_from_vpid()
by adding checks to return NULL when running in interrupt context or when
the current task has no active pid namespace.

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

[Severity: Medium]
Does the use of in_interrupt() here incorrectly reject valid task contexts
where bottom halves (BH) are disabled?

Since in_interrupt() evaluates to irq_count(), it evaluates to true not only
in hardware and softirq contexts, but also in normal process contexts if
local_bh_disable() or spin_lock_bh() is active. This might cause valid BPF
programs running in BH-disabled task contexts to unexpectedly receive NULL
from bpf_task_from_vpid().

Would it be better to use !in_task() here to accurately filter out non-task
asynchronous contexts without penalizing BH-disabled task contexts?

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

[ ... ]

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

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

* Re: [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-06  9:19 [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
  2026-06-06  9:31 ` sashiko-bot
@ 2026-06-06  9:58 ` bot+bpf-ci
  2026-06-06 12:57 ` Leon Hwang
  2026-06-07  8:44 ` Kumar Kartikeya Dwivedi
  3 siblings, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-06-06  9:58 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: 1470 bytes --]

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

This isn't a bug, but a review comment on v3 about this in_interrupt()
check does not appear to have been answered.

The comment noted that in_interrupt() evaluates to irq_count(), so it
returns true not only in hardware and softirq context but also in normal
task context whenever bottom halves are disabled, such as inside
local_bh_disable() or spin_lock_bh().

Would this cause bpf_task_from_vpid() to return NULL for a valid BPF
program running in a BH-disabled task context, where current still has a
valid pid namespace?

Would using !in_task() instead filter out the asynchronous contexts
without rejecting BH-disabled task contexts?

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

> +
>  	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);


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

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

* Re: [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-06  9:19 [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
  2026-06-06  9:31 ` sashiko-bot
  2026-06-06  9:58 ` bot+bpf-ci
@ 2026-06-06 12:57 ` Leon Hwang
  2026-06-07  8:44 ` Kumar Kartikeya Dwivedi
  3 siblings, 0 replies; 7+ messages in thread
From: Leon Hwang @ 2026-06-06 12:57 UTC (permalink / raw)
  To: Sechang Lim, 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

On 2026/6/6 17:19, Sechang Lim wrote:
[...]
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3c..226c31ccb5d6 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2912,7 +2912,14 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>  {
>  	struct task_struct *p;
>  
> +	if (in_interrupt())
> +		return NULL;
> +
>  	rcu_read_lock();

Better to use guard(rcu)() here, and drop the rcu_read_unlock().

Thanks,
Leon

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


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

* Re: [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid()
  2026-06-06  9:19 [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
                   ` (2 preceding siblings ...)
  2026-06-06 12:57 ` Leon Hwang
@ 2026-06-07  8:44 ` Kumar Kartikeya Dwivedi
  2026-06-07 10:05   ` Sechang Lim
  3 siblings, 1 reply; 7+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-06-07  8:44 UTC (permalink / raw)
  To: Sechang Lim, 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

On Sat Jun 6, 2026 at 11:19 AM CEST, 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 bpf_task_from_vpid() runs in interrupt
> context, or when current has no pid namespace.
>
> Acked-by: Yonghong Song <yonghong.song@linux.dev>
> Fixes: 675c3596ff32 ("bpf: Add bpf_task_from_vpid() kfunc")
> Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
> ---
> v3:
>  - Also handle current with no pid namespace
>
> v2:
>  - Reject calls from interrupt context (Yonghong Song)
>  - https://lore.kernel.org/bpf/20260605200501.1619406-1-rhkrqnwk98@gmail.com/
>
> v1:
>  - https://lore.kernel.org/bpf/20260603204206.773482-1-rhkrqnwk98@gmail.com/
>
>  kernel/bpf/helpers.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b5314c9fed3c..226c31ccb5d6 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2912,7 +2912,14 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>  {
>  	struct task_struct *p;
>
> +	if (in_interrupt())
> +		return NULL;
> +

This seems too broad, I would just drop this hunk. It seems unrelated to the fix.
IIUC we only need the bit below to prevent the original NULL deref.

pw-bot: cr

>  	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);


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

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

On Sun, Jun 07, 2026 at 10:44:41AM +0200, Kumar Kartikeya Dwivedi wrote:
>>  kernel/bpf/helpers.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index b5314c9fed3c..226c31ccb5d6 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2912,7 +2912,14 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>>  {
>>  	struct task_struct *p;
>>
>> +	if (in_interrupt())
>> +		return NULL;
>> +
>
>This seems too broad, I would just drop this hunk. It seems unrelated to the fix.
>IIUC we only need the bit below to prevent the original NULL deref.
>
>pw-bot: cr
>
>>  	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);
>

Right, the NULL check alone fixes the crash. The async-context guard was
added on Yonghong's v1 request: in softirq current is unrelated to the
packet, so the looked-up task is meaning less even without the crash.

Drop it entirely, or keep that intent with a narrower predicate?
in_interrupt() is also true under spin_lock_bh(), so !in_task() would be
more precise.


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

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

On Sun Jun 7, 2026 at 12:05 PM CEST, Sechang Lim wrote:
> On Sun, Jun 07, 2026 at 10:44:41AM +0200, Kumar Kartikeya Dwivedi wrote:
>>>  kernel/bpf/helpers.c | 7 +++++++
>>>  1 file changed, 7 insertions(+)
>>>
>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>> index b5314c9fed3c..226c31ccb5d6 100644
>>> --- a/kernel/bpf/helpers.c
>>> +++ b/kernel/bpf/helpers.c
>>> @@ -2912,7 +2912,14 @@ __bpf_kfunc struct task_struct *bpf_task_from_vpid(s32 vpid)
>>>  {
>>>  	struct task_struct *p;
>>>
>>> +	if (in_interrupt())
>>> +		return NULL;
>>> +
>>
>>This seems too broad, I would just drop this hunk. It seems unrelated to the fix.
>>IIUC we only need the bit below to prevent the original NULL deref.
>>
>>pw-bot: cr
>>
>>>  	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);
>>
>
> Right, the NULL check alone fixes the crash. The async-context guard was
> added on Yonghong's v1 request: in softirq current is unrelated to the
> packet, so the looked-up task is meaning less even without the crash.
>
> Drop it entirely, or keep that intent with a narrower predicate?
> in_interrupt() is also true under spin_lock_bh(), so !in_task() would be
> more precise.

Drop it. I think there are contexts where tracing programs use it, may run with
interrupts disabled, but current still remains meaningful.

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

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

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-06  9:19 [PATCH bpf v3] bpf: fix NULL pointer dereference in bpf_task_from_vpid() Sechang Lim
2026-06-06  9:31 ` sashiko-bot
2026-06-06  9:58 ` bot+bpf-ci
2026-06-06 12:57 ` Leon Hwang
2026-06-07  8:44 ` Kumar Kartikeya Dwivedi
2026-06-07 10:05   ` Sechang Lim
2026-06-07 11:06     ` Kumar Kartikeya Dwivedi

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.