Re: bpf: Is the nmi_uaccess_okay() check needed for bpf_send_signal_task()?

On Wed, Aug 19, 2026 at 5:44 AM Aditya Sharma <[email protected]> wrote:
>
> Hi,
>
> I hit this from a bpf_timer callback that signals a userspace task: the
> signal was not delivered some of the time. It turned out that
> bpf_send_signal_task() was returning -EPERM depending on which task the
> softirq happened to interrupt, and the source is this check in
> bpf_send_signal_common():
>
> bpf_trace.c:872:        if (unlikely(task->flags & (PF_KTHREAD | PF_EXITING)))
> bpf_trace.c:873:                return -EPERM;
> bpf_trace.c:874:        if (unlikely(!nmi_uaccess_okay()))
> bpf_trace.c:875:                return -EPERM;
>
> Is the nmi_uaccess_okay() check necessary in the bpf_send_signal_task()
> case?
>
> When commit 6280cf7 ("bpf: Implement bpf_send_signal_task() kfunc")
> made the function take a task, the PF_KTHREAD and is_global_init()
> started testing task, while nmi_uaccess_okay() kept testing current.

Yeah, this is weird, at the very least it should be checking it for
the provided task. But I also am not sure why we have to check this...

Yonghong, do you remember why this was necessary? Can we just drop this?

>
> As far as I can tell the path here does no user memory access, so
> there is nothing for the check to guard.
>
> Also, it compiles to true outside x86 (include/asm-generic/tlb.h:26),
> so if it were necessary here the kfunc would be broken on every other
> architecture.
>
> Thus, whether a signal can be sent depends on the caller's context,
> rather than the task it is to be sent to. And at the bpf_timer callback,
> we never know deterministically whether the signal would go through,
> or it would get rejected here.
>
> A quick reproducibility test by Claude Opus 5 (the issue I was facing
> from the timer callback context was not reproducible deterministically):
>
> SEC("tp_btf/workqueue_execute_start")   /* current is a kworker   */
> SEC("tp_btf/sys_enter")                 /* current is a user task */
>
> static __always_inline int probe(void)
> {
> struct task_struct *t;
> int ret;
>
> t = bpf_task_from_pid(target_pid);
> if (!t)
>   return -ESRCH;
> /* sig == 0: permission check only, nothing is delivered */
> ret = bpf_send_signal_task(t, 0, PIDTYPE_TGID, 0);
> bpf_task_release(t);
> return ret;
> }
>
> On bpf-next commit f79066c7 (selftests/bpf: Add tests for a store
> on a fault prone qdisc pointer), x86_64 under qemu:
>
> bpf_send_signal_task() from kworker context : -1  (-EPERM)
> bpf_send_signal_task() from syscall context :  0
>
> Same result on v6.18.29, x86_64, bare metal.
>
> I may be missing the reason for it to be there as well. If this check
> is intentional, what is it protecting in the _task case?
>
> Thanks,
> Aditya Sharma