signalfd stores its mask in the shared signalfd_ctx attached to the
open file description, but do_signalfd4() only wakes the caller's
signalfd_wqh after updating that shared mask.
If a signalfd file description is shared across fork(), another task
can block in read() or poll() with an old mask while a signal that does
not match that old mask is already pending. When the parent later
reconfigures the shared signalfd to a new mask that does match the
pending signal, the child can remain asleep indefinitely because the
reconfiguration never wakes the child's waitqueue.
At the same time, ctx->sigmask is shared file state, but it is read and
written under per-task sighand locks, which does not serialize
cross-process access to the shared file description.
Restore file-scoped synchronization for file-scoped state by adding a
lock and waitqueue to signalfd_ctx. Make readers and pollers wait on the
file-local waitqueue for mask updates while continuing to use the local
signalfd_wqh for signal delivery. Protect ctx->sigmask with the
file-local lock and wake the file-local waitqueue after reconfiguration.
Add a selftest which reproduces the missed wakeup with a shared
signalfd inherited across fork().
Fixes: b8fceee17a31 ("signalfd simplification")
Assisted-by: LLM (Codex, GPT-5)
Signed-off-by: Vu Nguyen Anh Khoa <[email protected]>
---
fs/signalfd.c | 45 ++-
tools/testing/selftests/signal/.gitignore | 1 +
tools/testing/selftests/signal/Makefile | 2 +-
.../selftests/signal/signalfd_mask_wake.c | 266 ++++++++++++++++++
4 files changed, 302 insertions(+), 12 deletions(-)
create mode 100644 tools/testing/selftests/signal/signalfd_mask_wake.c
diff --git a/fs/signalfd.c b/fs/signalfd.c
index dff53745e352..523c3a6dc565 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -27,6 +27,7 @@
#include <linux/kernel.h>
#include <linux/signal.h>
#include <linux/list.h>
+#include <linux/rcupdate.h>
#include <linux/anon_inodes.h>
#include <linux/signalfd.h>
#include <linux/syscalls.h>
@@ -39,12 +40,19 @@ void signalfd_cleanup(struct sighand_struct *sighand)
}
struct signalfd_ctx {
+ /* Serializes shared mask updates with file-local waiter wakeups. */
+ spinlock_t lock;
+ wait_queue_head_t wqh;
+ struct rcu_head rcu;
sigset_t sigmask;
};
static int signalfd_release(struct inode *inode, struct file *file)
{
- kfree(file->private_data);
+ struct signalfd_ctx *ctx = file->private_data;
+
+ wake_up_pollfree(&ctx->wqh);
+ kfree_rcu(ctx, rcu);
return 0;
}
@@ -53,14 +61,17 @@ static __poll_t signalfd_poll(struct file *file, poll_table
*wait)
struct signalfd_ctx *ctx = file->private_data;
__poll_t events = 0;
+ poll_wait(file, &ctx->wqh, wait);
poll_wait(file, ¤t->sighand->signalfd_wqh, wait);
- spin_lock_irq(¤t->sighand->siglock);
+ spin_lock_irq(&ctx->lock);
+ spin_lock(¤t->sighand->siglock);
if (next_signal(¤t->pending, &ctx->sigmask) ||
next_signal(¤t->signal->shared_pending,
&ctx->sigmask))
events |= EPOLLIN;
- spin_unlock_irq(¤t->sighand->siglock);
+ spin_unlock(¤t->sighand->siglock);
+ spin_unlock_irq(&ctx->lock);
return events;
}
@@ -157,8 +168,10 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx,
kernel_siginfo_t *info
enum pid_type type;
ssize_t ret;
DECLARE_WAITQUEUE(wait, current);
+ DECLARE_WAITQUEUE(ctx_wait, current);
- spin_lock_irq(¤t->sighand->siglock);
+ spin_lock_irq(&ctx->lock);
+ spin_lock(¤t->sighand->siglock);
ret = dequeue_signal(&ctx->sigmask, info, &type);
switch (ret) {
case 0:
@@ -167,10 +180,12 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx,
kernel_siginfo_t *info
ret = -EAGAIN;
fallthrough;
default:
- spin_unlock_irq(¤t->sighand->siglock);
+ spin_unlock(¤t->sighand->siglock);
+ spin_unlock_irq(&ctx->lock);
return ret;
}
+ add_wait_queue(&ctx->wqh, &ctx_wait);
add_wait_queue(¤t->sighand->signalfd_wqh, &wait);
for (;;) {
set_current_state(TASK_INTERRUPTIBLE);
@@ -181,13 +196,17 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx,
kernel_siginfo_t *info
ret = -ERESTARTSYS;
break;
}
- spin_unlock_irq(¤t->sighand->siglock);
+ spin_unlock(¤t->sighand->siglock);
+ spin_unlock_irq(&ctx->lock);
schedule();
- spin_lock_irq(¤t->sighand->siglock);
+ spin_lock_irq(&ctx->lock);
+ spin_lock(¤t->sighand->siglock);
}
- spin_unlock_irq(¤t->sighand->siglock);
+ spin_unlock(¤t->sighand->siglock);
+ spin_unlock_irq(&ctx->lock);
remove_wait_queue(¤t->sighand->signalfd_wqh, &wait);
+ remove_wait_queue(&ctx->wqh, &ctx_wait);
__set_current_state(TASK_RUNNING);
return ret;
@@ -232,7 +251,9 @@ static void signalfd_show_fdinfo(struct seq_file *m, struct
file *f)
struct signalfd_ctx *ctx = f->private_data;
sigset_t sigmask;
+ spin_lock_irq(&ctx->lock);
sigmask = ctx->sigmask;
+ spin_unlock_irq(&ctx->lock);
signotset(&sigmask);
render_sigset_t(m, "sigmask:\t", &sigmask);
}
@@ -268,6 +289,8 @@ static int do_signalfd4(int ufd, sigset_t *mask, int flags)
if (!ctx)
return -ENOMEM;
+ spin_lock_init(&ctx->lock);
+ init_waitqueue_head(&ctx->wqh);
ctx->sigmask = *mask;
fd = FD_ADD(flags & O_CLOEXEC,
@@ -286,11 +309,11 @@ static int do_signalfd4(int ufd, sigset_t *mask, int
flags)
ctx = fd_file(f)->private_data;
if (fd_file(f)->f_op != &signalfd_fops)
return -EINVAL;
- spin_lock_irq(¤t->sighand->siglock);
+ spin_lock_irq(&ctx->lock);
ctx->sigmask = *mask;
- spin_unlock_irq(¤t->sighand->siglock);
+ spin_unlock_irq(&ctx->lock);
- wake_up(¤t->sighand->signalfd_wqh);
+ wake_up(&ctx->wqh);
}
return ufd;
diff --git a/tools/testing/selftests/signal/.gitignore
b/tools/testing/selftests/signal/.gitignore
index 3f339865a3b6..5cac532d49ae 100644
--- a/tools/testing/selftests/signal/.gitignore
+++ b/tools/testing/selftests/signal/.gitignore
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
mangle_uc_sigmask
sas
+signalfd_mask_wake
diff --git a/tools/testing/selftests/signal/Makefile
b/tools/testing/selftests/signal/Makefile
index e0bf7058d19c..00f1e96c92c4 100644
--- a/tools/testing/selftests/signal/Makefile
+++ b/tools/testing/selftests/signal/Makefile
@@ -2,6 +2,6 @@
CFLAGS = -Wall
TEST_GEN_PROGS = mangle_uc_sigmask
TEST_GEN_PROGS += sas
+TEST_GEN_PROGS += signalfd_mask_wake
include ../lib.mk
-
diff --git a/tools/testing/selftests/signal/signalfd_mask_wake.c
b/tools/testing/selftests/signal/signalfd_mask_wake.c
new file mode 100644
index 000000000000..3e60424b9ec9
--- /dev/null
+++ b/tools/testing/selftests/signal/signalfd_mask_wake.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <poll.h>
+#include <signal.h>
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/signalfd.h>
+#include <sys/syscall.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include "kselftest.h"
+
+static int write_all(int fd, const void *buf, size_t len)
+{
+ const char *ptr = buf;
+
+ while (len) {
+ ssize_t ret = write(fd, ptr, len);
+
+ if (ret < 0) {
+ if (errno == EINTR)
+ continue;
+ return -1;
+ }
+
+ ptr += ret;
+ len -= ret;
+ }
+
+ return 0;
+}
+
+static int read_all(int fd, void *buf, size_t len)
+{
+ char *ptr = buf;
+
+ while (len) {
+ ssize_t ret = read(fd, ptr, len);
+
+ if (ret < 0) {
+ if (errno == EINTR)
+ continue;
+ return -1;
+ }
+ if (ret == 0)
+ return -1;
+
+ ptr += ret;
+ len -= ret;
+ }
+
+ return 0;
+}
+
+static void kill_and_reap(pid_t pid)
+{
+ int status;
+
+ if (pid <= 0)
+ return;
+
+ kill(pid, SIGKILL);
+ waitpid(pid, &status, 0);
+}
+
+static int wait_result(int fd, int timeout_ms)
+{
+ struct pollfd pfd = {
+ .fd = fd,
+ .events = POLLIN,
+ };
+ int ret;
+
+ do {
+ ret = poll(&pfd, 1, timeout_ms);
+ } while (ret < 0 && errno == EINTR);
+
+ return ret;
+}
+
+static int child_in_signalfd_read(pid_t pid, int sfd)
+{
+ char path[64];
+ char buf[256];
+ long nr;
+ unsigned long arg0;
+ FILE *fp;
+ int in_read = 0;
+
+ snprintf(path, sizeof(path), "/proc/%d/syscall", pid);
+ fp = fopen(path, "r");
+ if (!fp) {
+ if (errno == ENOENT)
+ return -1;
+ return 0;
+ }
+
+ if (fgets(buf, sizeof(buf), fp) &&
+ sscanf(buf, "%ld %lx", &nr, &arg0) == 2 &&
+ nr == __NR_read && arg0 == (unsigned long)sfd)
+ in_read = 1;
+
+ fclose(fp);
+ return in_read;
+}
+
+static int wait_for_child_block(pid_t pid, int sfd, int timeout_ms)
+{
+ int waited_ms = 0;
+ int ret;
+
+ while (waited_ms < timeout_ms) {
+ ret = child_in_signalfd_read(pid, sfd);
+ if (ret < 0)
+ return ret;
+ if (ret > 0) {
+ usleep(10000);
+ return 0;
+ }
+
+ usleep(1000);
+ waited_ms++;
+ }
+
+ return -1;
+}
+
+int main(void)
+{
+ struct signalfd_siginfo fdsi;
+ sigset_t blocked, initial_mask, updated_mask;
+ int ready_pipe[2], start_pipe[2], result_pipe[2];
+ int sfd, signo, status;
+ pid_t child;
+ char ready, start;
+
+ ksft_print_header();
+ ksft_set_plan(1);
+
+ sigemptyset(&blocked);
+ sigaddset(&blocked, SIGUSR1);
+ sigaddset(&blocked, SIGUSR2);
+ if (sigprocmask(SIG_BLOCK, &blocked, NULL))
+ ksft_exit_fail_perror("sigprocmask");
+
+ sigemptyset(&initial_mask);
+ sigaddset(&initial_mask, SIGUSR1);
+ sfd = signalfd(-1, &initial_mask, 0);
+ if (sfd < 0)
+ ksft_exit_fail_perror("signalfd");
+
+ if (pipe(ready_pipe))
+ ksft_exit_fail_perror("pipe(ready)");
+ if (pipe(start_pipe))
+ ksft_exit_fail_perror("pipe(start)");
+ if (pipe(result_pipe))
+ ksft_exit_fail_perror("pipe(result)");
+
+ child = fork();
+ if (child < 0)
+ ksft_exit_fail_perror("fork");
+
+ if (child == 0) {
+ close(ready_pipe[0]);
+ close(start_pipe[1]);
+ close(result_pipe[0]);
+
+ ready = 'R';
+ if (write_all(ready_pipe[1], &ready, sizeof(ready)))
+ _exit(EXIT_FAILURE);
+
+ if (read_all(start_pipe[0], &start, sizeof(start)))
+ _exit(EXIT_FAILURE);
+
+ memset(&fdsi, 0, sizeof(fdsi));
+ if (read_all(sfd, &fdsi, sizeof(fdsi)))
+ _exit(EXIT_FAILURE);
+
+ signo = fdsi.ssi_signo;
+ if (write_all(result_pipe[1], &signo, sizeof(signo)))
+ _exit(EXIT_FAILURE);
+
+ _exit(EXIT_SUCCESS);
+ }
+
+ close(ready_pipe[1]);
+ close(start_pipe[0]);
+ close(result_pipe[1]);
+
+ if (read_all(ready_pipe[0], &ready, sizeof(ready))) {
+ kill_and_reap(child);
+ ksft_exit_fail_msg("child did not reach blocking read\n");
+ }
+
+ if (kill(child, SIGUSR2)) {
+ kill_and_reap(child);
+ ksft_exit_fail_perror("kill(SIGUSR2)");
+ }
+
+ start = 'S';
+ if (write_all(start_pipe[1], &start, sizeof(start))) {
+ kill_and_reap(child);
+ ksft_exit_fail_msg("could not release child into signalfd
read\n");
+ }
+
+ status = wait_for_child_block(child, sfd, 1000);
+ if (status < 0) {
+ kill_and_reap(child);
+ if (status == -1)
+ ksft_exit_skip("/proc/<pid>/syscall is unavailable\n");
+ ksft_exit_fail_msg("child did not block in signalfd read\n");
+ }
+
+ sigemptyset(&updated_mask);
+ sigaddset(&updated_mask, SIGUSR2);
+ if (signalfd(sfd, &updated_mask, 0) < 0) {
+ kill_and_reap(child);
+ ksft_exit_fail_perror("signalfd(reconfigure)");
+ }
+
+ if (wait_result(result_pipe[0], 1000) == 1) {
+ if (read_all(result_pipe[0], &signo, sizeof(signo))) {
+ kill_and_reap(child);
+ ksft_exit_fail_msg("child wakeup did not carry a
signal\n");
+ }
+
+ if (waitpid(child, &status, 0) != child) {
+ kill_and_reap(child);
+ ksft_exit_fail_perror("waitpid");
+ }
+ ksft_test_result(signo == SIGUSR2 && WIFEXITED(status) &&
+ WEXITSTATUS(status) == 0,
+ "shared signalfd wakeup after mask update\n");
+ ksft_exit_pass();
+ }
+
+ /*
+ * Buggy kernels leave the child asleep until an unrelated signal hits
+ * the child's signalfd waitqueue. SIGUSR1 does not match the updated
+ * signalfd mask, so a successful read of SIGUSR2 after this nudge
+ * demonstrates the missed wakeup.
+ */
+ if (kill(child, SIGUSR1)) {
+ kill_and_reap(child);
+ ksft_exit_fail_perror("kill(SIGUSR1)");
+ }
+
+ if (wait_result(result_pipe[0], 1000) == 1 &&
+ !read_all(result_pipe[0], &signo, sizeof(signo)) &&
+ waitpid(child, &status, 0) == child &&
+ signo == SIGUSR2 && WIFEXITED(status) && WEXITSTATUS(status) == 0) {
+ ksft_test_result_fail("mask update missed a shared signalfd
waiter\n");
+ ksft_print_msg("child woke only after an unrelated SIGUSR1\n");
+ ksft_exit_fail();
+ }
+
+ kill_and_reap(child);
+ ksft_exit_fail_msg("child remained blocked after signalfd
reconfiguration\n");
+}
--
2.43.0