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

The check was carried over from bpf_probe_write_user() when
bpf_send_signal() was added in commit 8b401f9ed244 ("bpf: implement
bpf_send_signal() helper"). There it guards a user memory write, but
bpf_send_signal_common() does no user memory access, so the check has
nothing to guard here. It is also already a no-op outside x86.

Remove it, and drop the part of the comment above it that described
the memory access checks, as none remain.

Fixes: 6280cf718db0 ("bpf: Implement bpf_send_signal_task() kfunc")
Suggested-by: Andrii Nakryiko <[email protected]>
Link: 
https://lore.kernel.org/bpf/[email protected]/T/#u
Signed-off-by: Aditya Sharma <[email protected]>
---
 kernel/trace/bpf_trace.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 195f78db9bda..f6e335a37a91 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -864,15 +864,12 @@ static int bpf_send_signal_common(u32 sig, enum pid_type 
type, struct task_struc
                siginfo = &info;
        }
 
-       /* 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.
+       /*
+        * Similar to bpf_probe_write_user, task needs to be
+        * in a sound condition.
         */
        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;
-- 
2.34.1


Reply via email to