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

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

[Aditya Sharma]

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.

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

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

[Andrii Nakryiko]

On Wed, Aug 19, 2026 at 5:44 AM Aditya Sharma <adi.sharma@zohomail.in> 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

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

[Yonghong Song]

On 8/21/26 11:21 AM, Andrii Nakryiko wrote:
> On Wed, Aug 19, 2026 at 5:44 AM Aditya Sharma <adi.sharma@zohomail.in> 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?

I went through the history.
In 5.3: we have bpf_send_signal() helper:

BPF_CALL_1(bpf_send_signal, u32, sig)
{
	...
         /* Similar to bpf_probe_write_user, task needs to be
          * in a sound condition and kernel memory access be
          * permitted in order to send signal to the current
          * task.
          */
         if (unlikely(current->flags & (PF_KTHREAD | PF_EXITING)))
                 return -EPERM;
         if (unlikely(uaccess_kernel()))
                 return -EPERM;
         if (unlikely(!nmi_uaccess_okay()))
                 return -EPERM;
	...
}

nmi_uaccess_okay() is needed for *current* task.

In 5.6, bpf_send_signal_thread() is added, but still for the same task,
so we still have:

static int bpf_send_signal_common(u32 sig, enum pid_type type)
{
	...
         /* Similar to bpf_probe_write_user, task needs to be
          * in a sound condition and kernel memory access be
          * permitted in order to send signal to the current
          * task.
          */
         if (unlikely(current->flags & (PF_KTHREAD | PF_EXITING)))
                 return -EPERM;
         if (unlikely(uaccess_kernel()))
                 return -EPERM;
         if (unlikely(!nmi_uaccess_okay()))
                 return -EPERM;
	...
}

In the current master:

__bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid_type type,
                                      u64 value)
{
         if (type != PIDTYPE_PID && type != PIDTYPE_TGID)
                 return -EINVAL;

         return bpf_send_signal_common(sig, type, task, value);
}

static int bpf_send_signal_common(u32 sig, enum pid_type type, struct task_struct *task, u64 value)
{
	...
         /* Similar to bpf_probe_write_user, task needs to be
          * in a sound condition and kernel memory access be
          * permitted in order to send signal to the current
          * task.
          */
         if (unlikely(task->flags & (PF_KTHREAD | PF_EXITING)))
                 return -EPERM;
         if (unlikely(!nmi_uaccess_okay()))
                 return -EPERM;
         /* Task should not be pid=1 to avoid kernel panic. */
         if (unlikely(is_global_init(task)))
                 return -EPERM;
	...
}

We probably cannot remove
         if (unlikely(!nmi_uaccess_okay()))
                 return -EPERM;
unconditionally. The above 'if' statement should be guarded
with *current* task if it is different from parameter "task"
since nmi_uaccess_okay() is only for the *current* task.

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