Re: [PATCH bpf-next 1/2] bpf: Only check nmi_uaccess_okay() when signalling current

[Andrii Nakryiko]

On Wed, Sep 2, 2026 at 8:05 AM Aditya Sharma <adi.sharma@xxxxxxxxxxx> wrote:
>
> nmi_uaccess_okay() takes no task argument and is a statement about
> current. When commit 6280cf718db0 ("bpf: Implement bpf_send_signal_task()
> kfunc") made the PF_KTHREAD/PF_EXITING and is_global_init() test the
> supplied task, nmi_uaccess_okay() was left testing current.
>
> As a result bpf_send_signal_task() returns -EPERM whenever the
> calling context happens to be a kernel thread, regardless of which task
> the signal is aimed at. The same call with the same target succeeds or
> fails depending only on what the CPU was running:
>
>   bpf_send_signal_task() from tp_btf/workqueue_execute_start : -EPERM
>   bpf_send_signal_task() from tp_btf/sys_enter               : 0
>
> This was originally hit from a bpf_timer callback, where the failure
> is intermittent because a softirq runs on whichever task it
> interrupted. The rejection is x86-only, as nmi_uaccess_okay() is
> defined as true in include/asm-generic/tlb.h elsewhere.
>
> Only apply the check when the signal is sent to current, where the
> predicate is meaningful. bpf_send_signal() and bpf_send_signal_thread()
> assign task = current, so their behaviour is unchanged.
>
> Fixes: 6280cf718db0 ("bpf: Implement bpf_send_signal_task() kfunc")
> Suggested-by: Yonghong Song <yonghong.song@xxxxxxxxx>
> Link: https://lore.kernel.org/bpf/20260819124324.43162-1-adi.sharma@xxxxxxxxxxx/T/#u
>
> Signed-off-by: Aditya Sharma <adi.sharma@xxxxxxxxxxx>
> ---
>  kernel/trace/bpf_trace.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 29260951aa87..f7a41f222599 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -871,7 +871,10 @@ static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struc
>          */
>         if (unlikely(task->flags & (PF_KTHREAD | PF_EXITING)))
>                 return -EPERM;
> -       if (unlikely(!nmi_uaccess_okay()))
> +       /* Since nmi_uaccess_okay() is only for the current
> +        * task, check if task is current.
> +        */
> +       if (task == current && unlikely(!nmi_uaccess_okay()))
>                 return -EPERM;

I think we should just drop nmi_uaccess_okay() check altogether.
Unless there is some technical reason why **for current task only** we
can't send signals, there is no good reason to have this.

let's just drop, it seems like just a copy/paste from some user memory
probing logic, which shouldn't apply for signals.

pw-bot: cr

>         /* Task should not be pid=1 to avoid kernel panic. */
>         if (unlikely(is_global_init(task)))
>
> base-commit: d761934c9483ecde93fe99d8705282f716dfee50
> --
> 2.34.1
>