Re: [PATCH bpf v2] bpf: fix NULL pointer dereference in bpf_task_from_vpid()

[sashiko-bot@kernel.org]

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