[PATCH] signalfd: wake shared signalfd waiters after mask updates

Vu Nguyen Anh Khoa posted 1 patch 3 weeks, 4 days ago
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
[PATCH] signalfd: wake shared signalfd waiters after mask updates
Posted by Vu Nguyen Anh Khoa 3 weeks, 4 days ago
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 <khoavna.tin.2225@gmail.com>
---
 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, &current->sighand->signalfd_wqh, wait);
 
-	spin_lock_irq(&current->sighand->siglock);
+	spin_lock_irq(&ctx->lock);
+	spin_lock(&current->sighand->siglock);
 	if (next_signal(&current->pending, &ctx->sigmask) ||
 	    next_signal(&current->signal->shared_pending,
 			&ctx->sigmask))
 		events |= EPOLLIN;
-	spin_unlock_irq(&current->sighand->siglock);
+	spin_unlock(&current->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(&current->sighand->siglock);
+	spin_lock_irq(&ctx->lock);
+	spin_lock(&current->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(&current->sighand->siglock);
+		spin_unlock(&current->sighand->siglock);
+		spin_unlock_irq(&ctx->lock);
 		return ret;
 	}
 
+	add_wait_queue(&ctx->wqh, &ctx_wait);
 	add_wait_queue(&current->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(&current->sighand->siglock);
+		spin_unlock(&current->sighand->siglock);
+		spin_unlock_irq(&ctx->lock);
 		schedule();
-		spin_lock_irq(&current->sighand->siglock);
+		spin_lock_irq(&ctx->lock);
+		spin_lock(&current->sighand->siglock);
 	}
-	spin_unlock_irq(&current->sighand->siglock);
+	spin_unlock(&current->sighand->siglock);
+	spin_unlock_irq(&ctx->lock);
 
 	remove_wait_queue(&current->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(&current->sighand->siglock);
+		spin_lock_irq(&ctx->lock);
 		ctx->sigmask = *mask;
-		spin_unlock_irq(&current->sighand->siglock);
+		spin_unlock_irq(&ctx->lock);
 
-		wake_up(&current->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