* [PATCH bpf-next 2/2] selftests/bpf: Test bpf_send_signal_task() from kernel thread context
2026-09-02 15:04 [PATCH bpf-next 1/2] bpf: Only check nmi_uaccess_okay() when signalling current Aditya Sharma
@ 2026-09-02 15:04 ` Aditya Sharma
2026-09-02 15:18 ` sashiko-bot
2026-09-02 15:14 ` [PATCH bpf-next 1/2] bpf: Only check nmi_uaccess_okay() when signalling current sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Aditya Sharma @ 2026-09-02 15:04 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, martin.lau, yonghong.song, eddyz87, song,
jolsa, kpsingh, shuah, puranjay, linux-kernel, linux-kselftest,
Aditya Sharma
The existing send_signal tests, including the _remote variants that
exercise bpf_send_signal_task() with a caller-supplied target, all
gate on
if ((bpf_get_current_pid_tgid() >> 32) == pid)
so the BPF program only ever runs while current is the test process.
The caller-context dimension is therefore untested, which is how
bpf_send_signal_task() failing from kernel thread context went
unnoticed.
Add a program attached to tp_btf/workqueue_execute_start, where
current is always a kworker. It cannot filter on current, so it is
driven by target_pid alone, and the test asserts the child receives
the signal. Setting pid to 0 leaves the pre-existing programs in the
skeleton inert. Workqueues run on their own, so nothing has to be
triggered from userspace.
Without narrowing down the nmi_uaccess_okay() check to task == current,
this test fails on x86.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Aditya Sharma <adi.sharma@zohomail.in>
---
.../selftests/bpf/prog_tests/send_signal.c | 104 ++++++++++++++++++
.../bpf/progs/test_send_signal_kern.c | 26 +++++
2 files changed, 130 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/send_signal.c b/tools/testing/selftests/bpf/prog_tests/send_signal.c
index 7ac4d5a488aa..f9dd23db1ef9 100644
--- a/tools/testing/selftests/bpf/prog_tests/send_signal.c
+++ b/tools/testing/selftests/bpf/prog_tests/send_signal.c
@@ -261,6 +261,108 @@ static void test_send_signal_nmi(bool signal_thread, bool remote)
test_send_signal_common(&attr, signal_thread, remote);
}
+static void test_send_signal_kworker(void)
+{
+ struct test_send_signal_kern *skel;
+ int pipe_c2p[2], pipe_p2c[2];
+ struct sigaction sa = {};
+ char buf[256];
+ int err = -1;
+ pid_t pid;
+
+ if (!ASSERT_OK(pipe(pipe_c2p), "pipe_c2p"))
+ return;
+
+ if (!ASSERT_OK(pipe(pipe_p2c), "pipe_p2c")) {
+ close(pipe_c2p[0]);
+ close(pipe_c2p[1]);
+ return;
+ }
+
+ pid = fork();
+ if (!ASSERT_GE(pid, 0, "fork")) {
+ close(pipe_c2p[0]);
+ close(pipe_c2p[1]);
+ close(pipe_p2c[0]);
+ close(pipe_p2c[1]);
+ return;
+ }
+
+ if (pid == 0) {
+ /* install signal handler and notify parent */
+ sa.sa_sigaction = sigusr1_siginfo_handler;
+ sa.sa_flags = SA_RESTART | SA_SIGINFO;
+ ASSERT_NEQ(sigaction(SIGUSR1, &sa, NULL), -1, "sigaction");
+
+ close(pipe_c2p[0]); /* close read */
+ close(pipe_p2c[1]); /* close write */
+
+ /* notify parent signal handler is installed */
+ ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write");
+
+ /* make sure parent enabled bpf program to send_signal */
+ ASSERT_EQ(read(pipe_p2c[0], buf, 1), 1, "pipe_read");
+
+ /* the signal is sent from workqueue context, so nothing has
+ * to be triggered from here
+ */
+ while (!sigusr1_received)
+ sleep(1);
+
+ buf[0] = sigusr1_received;
+
+ ASSERT_EQ(sigusr1_received, 8, "sigusr1_received");
+ ASSERT_EQ(write(pipe_c2p[1], buf, 1), 1, "pipe_write");
+
+ close(pipe_c2p[1]);
+ close(pipe_p2c[0]);
+ exit(0);
+ }
+
+ close(pipe_c2p[1]); /* close write */
+ close(pipe_p2c[0]); /* close read */
+
+ skel = test_send_signal_kern__open_and_load();
+ if (!ASSERT_OK_PTR(skel, "skel_open_and_load"))
+ goto skel_open_load_failure;
+
+ /* wait until child signal handler installed */
+ ASSERT_EQ(read(pipe_c2p[0], buf, 1), 1, "pipe_read");
+
+ /* pid == 0 keeps the other programs of this skeleton inactive */
+ skel->bss->pid = 0;
+ skel->bss->sig = SIGUSR1;
+ skel->bss->target_pid = pid;
+
+ err = test_send_signal_kern__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach")) {
+ err = -1;
+ goto destroy_skel;
+ }
+
+ /* notify child that bpf program can send_signal now */
+ ASSERT_EQ(write(pipe_p2c[1], buf, 1), 1, "pipe_write");
+
+ /* wait for result, workqueues run on their own */
+ err = read_with_timeout(pipe_c2p[0], buf, 1, 10 * 1000 * 1000);
+ if (!ASSERT_GT(err, 0, "reading pipe"))
+ goto destroy_skel;
+
+ ASSERT_EQ(buf[0], 8, "incorrect result");
+
+destroy_skel:
+ test_send_signal_kern__destroy(skel);
+skel_open_load_failure:
+ close(pipe_c2p[0]);
+ close(pipe_p2c[1]);
+ /*
+ * Child is either about to exit cleanly or stuck in case of errors.
+ * Nudge it to exit.
+ */
+ kill(pid, SIGKILL);
+ wait(NULL);
+}
+
void test_send_signal(void)
{
if (test__start_subtest("send_signal_tracepoint"))
@@ -289,4 +391,6 @@ void test_send_signal(void)
test_send_signal_perf(true, true);
if (test__start_subtest("send_signal_nmi_thread_remote"))
test_send_signal_nmi(true, true);
+ if (test__start_subtest("send_signal_kworker"))
+ test_send_signal_kworker();
}
diff --git a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
index 176a355e3062..d1c000c30885 100644
--- a/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_send_signal_kern.c
@@ -60,6 +60,32 @@ int send_signal_tp_sched(void *ctx)
return bpf_send_signal_test(ctx);
}
+/* Send a signal to a task other than current, from a context where current
+ * is a kernel thread. No filtering on current is possible here, so this is
+ * driven entirely by target_pid.
+ */
+SEC("tp_btf/workqueue_execute_start")
+int send_signal_kworker(void *ctx)
+{
+ struct task_struct *target_task;
+ int ret;
+
+ if (status != 0 || target_pid == 0)
+ return 0;
+
+ target_task = bpf_task_from_pid(target_pid);
+ if (!target_task)
+ return 0;
+
+ ret = bpf_send_signal_task(target_task, sig, PIDTYPE_TGID, 8);
+ bpf_task_release(target_task);
+
+ if (ret == 0)
+ status = 1;
+
+ return 0;
+}
+
SEC("perf_event")
int send_signal_perf(void *ctx)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH bpf-next 1/2] bpf: Only check nmi_uaccess_okay() when signalling current
2026-09-02 15:04 [PATCH bpf-next 1/2] bpf: Only check nmi_uaccess_okay() when signalling current Aditya Sharma
2026-09-02 15:04 ` [PATCH bpf-next 2/2] selftests/bpf: Test bpf_send_signal_task() from kernel thread context Aditya Sharma
@ 2026-09-02 15:14 ` sashiko-bot
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-09-02 15:14 UTC (permalink / raw)
To: Aditya Sharma; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] Multi-line comment does not follow BPF subsystem standards.
Pre-existing issues:
- [High] A TOCTOU data race on the per-CPU `send_signal_work` structure allows NMI reentrancy to corrupt irq_work data and leak a task_struct reference.
--
commit b0fb28992ab88d35a628c6d9650cd4ab7e67c397
Author: Aditya Sharma <adi.sharma@zohomail.in>
bpf: Only check nmi_uaccess_okay() when signalling current
nmi_uaccess_okay() takes no task argument and is a statement about current.
This commit restricts the check to only apply when signalling the current
task, fixing a bug where bpf_send_signal_task() returned -EPERM for kernel
threads.
Link: https://lore.kernel.org/bpf/20260819124324.43162-1-adi.sharma@zohomail.in/T/#u
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index 29260951aa871..f7a41f2225999 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.
> + */
[Severity: Low]
This isn't a bug, but does this newly added comment violate the BPF subsystem
multi-line comment style?
The BPF subsystem strictly requires multi-line comments to have the opening
/* on its own line, with the text starting on the next line.
> + if (task == current && unlikely(!nmi_uaccess_okay()))
> return -EPERM;
> /* Task should not be pid=1 to avoid kernel panic. */
> if (unlikely(is_global_init(task)))
[Severity: High]
This is a pre-existing issue, but is there a TOCTOU data race on the per-CPU
send_signal_work structure later in bpf_send_signal_common()?
If a BPF program executing in a context with IRQs disabled calls
bpf_send_signal(), it evaluates irq_work_is_busy() as false. Before it
queues the work, an NMI could fire on the same CPU and also call
bpf_send_signal():
kernel/trace/bpf_trace.c:bpf_send_signal_common() {
...
if (preempt_count() != 0 || irqs_disabled()) {
...
work = this_cpu_ptr(&send_signal_work);
if (irq_work_is_busy(&work->irq_work))
return -EBUSY;
/* NMI fires here, calls bpf_send_signal_common(), evaluating
* irq_work_is_busy() as false since it's not queued yet.
* NMI writes its data and queues the work.
*/
work->task = get_task_struct(task);
work->has_siginfo = siginfo == &info;
if (work->has_siginfo)
copy_siginfo(&work->info, &info);
work->sig = sig;
work->type = type;
irq_work_queue(&work->irq_work);
return 0;
}
...
}
Upon return from the NMI, the interrupted context resumes, overwrites
work->task with a new get_task_struct(), and fails to queue the work since
the NMI already queued it.
Could this allow NMI reentrancy to corrupt irq_work data and permanently leak
a task_struct reference?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260902150439.692725-1-adi.sharma@zohomail.in?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread