[PATCH] signalfd: don't dequeue the forced fatal signals

[PATCH] signalfd: don't dequeue the forced fatal signals

[Oleg Nesterov]

These signals should act like SIGKILL, in that userspace must never dequeue
them. But as Kusaram explains, io_uring-driven signalfd_read_iter() called
from get_signal() -> task_work_run() paths can do this before get_signal()
has a chance to dequeue such a signal and notice SA_IMMUTABLE.

Change signalfd_poll() and signalfd_dequeue() to add pending SA_IMMUTABLE
signals to ctx->sigmask.

Cc: stable@kernel.org
Reported-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0a4c46806941297fecb9
Tested-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/69d122fd.050a0220.2dbe29.001c.GAE@google.com/
Suggested-by: Kusaram Devineni <kusaram@devineni.in>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/signalfd.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index dff53745e352..107a83336657 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -48,17 +48,30 @@ static int signalfd_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static void mk_sigmask(struct signalfd_ctx *ctx, sigset_t *sigmask)
+{
+	struct k_sigaction *k = current->sighand->action;
+	int n;
+
+	*sigmask = ctx->sigmask;
+	for (n = 1; n <= _NSIG; ++n, ++k) {
+		if (k->sa.sa_flags & SA_IMMUTABLE)
+			sigaddset(sigmask, n);
+	}
+}
+
 static __poll_t signalfd_poll(struct file *file, poll_table *wait)
 {
 	struct signalfd_ctx *ctx = file->private_data;
 	__poll_t events = 0;
+	sigset_t sigmask;
 
 	poll_wait(file, &current->sighand->signalfd_wqh, wait);
 
 	spin_lock_irq(&current->sighand->siglock);
-	if (next_signal(&current->pending, &ctx->sigmask) ||
-	    next_signal(&current->signal->shared_pending,
-			&ctx->sigmask))
+	mk_sigmask(ctx, &sigmask);
+	if (next_signal(&current->pending, &sigmask) ||
+	    next_signal(&current->signal->shared_pending, &sigmask))
 		events |= EPOLLIN;
 	spin_unlock_irq(&current->sighand->siglock);
 
@@ -155,11 +168,13 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 				int nonblock)
 {
 	enum pid_type type;
-	ssize_t ret;
 	DECLARE_WAITQUEUE(wait, current);
+	sigset_t sigmask;
+	ssize_t ret;
 
 	spin_lock_irq(&current->sighand->siglock);
-	ret = dequeue_signal(&ctx->sigmask, info, &type);
+	mk_sigmask(ctx, &sigmask);
+	ret = dequeue_signal(&sigmask, info, &type);
 	switch (ret) {
 	case 0:
 		if (!nonblock)
@@ -174,7 +189,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 	add_wait_queue(&current->sighand->signalfd_wqh, &wait);
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
-		ret = dequeue_signal(&ctx->sigmask, info, &type);
+		ret = dequeue_signal(&sigmask, info, &type);
 		if (ret != 0)
 			break;
 		if (signal_pending(current)) {
@@ -184,6 +199,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 		spin_unlock_irq(&current->sighand->siglock);
 		schedule();
 		spin_lock_irq(&current->sighand->siglock);
+		mk_sigmask(ctx, &sigmask);
 	}
 	spin_unlock_irq(&current->sighand->siglock);
 
-- 
2.52.0

Re: [PATCH] signalfd: don't dequeue the forced fatal signals

[Kees Cook]

On Sun, Apr 05, 2026 at 06:09:21PM +0200, Oleg Nesterov wrote:
> These signals should act like SIGKILL, in that userspace must never dequeue
> them. But as Kusaram explains, io_uring-driven signalfd_read_iter() called
> from get_signal() -> task_work_run() paths can do this before get_signal()
> has a chance to dequeue such a signal and notice SA_IMMUTABLE.
> 
> Change signalfd_poll() and signalfd_dequeue() to add pending SA_IMMUTABLE
> signals to ctx->sigmask.
> 
> Cc: stable@kernel.org
> Reported-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0a4c46806941297fecb9
> Tested-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
> Link: https://lore.kernel.org/all/69d122fd.050a0220.2dbe29.001c.GAE@google.com/
> Suggested-by: Kusaram Devineni <kusaram@devineni.in>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Reviewed-by: Kees Cook <kees@kernel.org>

Who should take this? I'm happy to add it to my seccomp tree if akpm (or
maybe Christian wants it)?

-Kees

> ---
>  fs/signalfd.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/signalfd.c b/fs/signalfd.c
> index dff53745e352..107a83336657 100644
> --- a/fs/signalfd.c
> +++ b/fs/signalfd.c
> @@ -48,17 +48,30 @@ static int signalfd_release(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> +static void mk_sigmask(struct signalfd_ctx *ctx, sigset_t *sigmask)
> +{
> +	struct k_sigaction *k = current->sighand->action;
> +	int n;
> +
> +	*sigmask = ctx->sigmask;
> +	for (n = 1; n <= _NSIG; ++n, ++k) {
> +		if (k->sa.sa_flags & SA_IMMUTABLE)
> +			sigaddset(sigmask, n);
> +	}
> +}
> +
>  static __poll_t signalfd_poll(struct file *file, poll_table *wait)
>  {
>  	struct signalfd_ctx *ctx = file->private_data;
>  	__poll_t events = 0;
> +	sigset_t sigmask;
>  
>  	poll_wait(file, &current->sighand->signalfd_wqh, wait);
>  
>  	spin_lock_irq(&current->sighand->siglock);
> -	if (next_signal(&current->pending, &ctx->sigmask) ||
> -	    next_signal(&current->signal->shared_pending,
> -			&ctx->sigmask))
> +	mk_sigmask(ctx, &sigmask);
> +	if (next_signal(&current->pending, &sigmask) ||
> +	    next_signal(&current->signal->shared_pending, &sigmask))
>  		events |= EPOLLIN;
>  	spin_unlock_irq(&current->sighand->siglock);
>  
> @@ -155,11 +168,13 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
>  				int nonblock)
>  {
>  	enum pid_type type;
> -	ssize_t ret;
>  	DECLARE_WAITQUEUE(wait, current);
> +	sigset_t sigmask;
> +	ssize_t ret;
>  
>  	spin_lock_irq(&current->sighand->siglock);
> -	ret = dequeue_signal(&ctx->sigmask, info, &type);
> +	mk_sigmask(ctx, &sigmask);
> +	ret = dequeue_signal(&sigmask, info, &type);
>  	switch (ret) {
>  	case 0:
>  		if (!nonblock)
> @@ -174,7 +189,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
>  	add_wait_queue(&current->sighand->signalfd_wqh, &wait);
>  	for (;;) {
>  		set_current_state(TASK_INTERRUPTIBLE);
> -		ret = dequeue_signal(&ctx->sigmask, info, &type);
> +		ret = dequeue_signal(&sigmask, info, &type);
>  		if (ret != 0)
>  			break;
>  		if (signal_pending(current)) {
> @@ -184,6 +199,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
>  		spin_unlock_irq(&current->sighand->siglock);
>  		schedule();
>  		spin_lock_irq(&current->sighand->siglock);
> +		mk_sigmask(ctx, &sigmask);
>  	}
>  	spin_unlock_irq(&current->sighand->siglock);
>  
> -- 
> 2.52.0
> 
> 

-- 
Kees Cook

Re: [PATCH] signalfd: don't dequeue the forced fatal signals

[Oleg Nesterov]

On 04/05, Kees Cook wrote:
>
> Reviewed-by: Kees Cook <kees@kernel.org>

Thanks!

> Who should take this? I'm happy to add it to my seccomp tree if akpm (or
> maybe Christian wants it)?

I am obviously fine either way, but if nobody objects I'd prefer your tree.

To remind, we have another (slightly related) problem,
[RFC PATCH] ptrace: don't report syscall-exit if the tracee was killed by seccomp
https://lore.kernel.org/all/adKGb5vkyggMK-_l@redhat.com/
I still hope to send V2 "soon" ;)

This is certainly the seccomp material, so I think it would be better to route
both changes via the same tree. But again, I am fine either way, this is minor.

But. I forgot to add the "TODO" note into the changelog. And mk_sigmask() is
not a good name... I'll send V2 with these (cosmetic) changes in a minute,
I'll preserve your ACK.

Oleg.

[PATCH v2] signalfd: don't dequeue the forced fatal signals

[Oleg Nesterov]

These signals should act like SIGKILL, in that userspace must never dequeue
them. But as Kusaram explains, io_uring-driven signalfd_read_iter() called
from get_signal() -> task_work_run() paths can do this before get_signal()
has a chance to dequeue such a signal and notice SA_IMMUTABLE.

Change signalfd_poll() and signalfd_dequeue() to add pending SA_IMMUTABLE
signals to ctx->sigmask.

TODO: we should probably change force_sig_info_to_task(HANDLER_EXIT) to
make fatal_signal_pending() true, or add a fatal_or_forced_signal_pending()
helper. Then signalfd_dequeue() could just return -EINTR in this case.
This also makes sense for get_signal(), which could prioritize a fatal
signal sent by (say) force_sig_seccomp(force_coredump => true), just like
it already prioritizes SIGKILL.

Cc: stable@kernel.org
Reported-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0a4c46806941297fecb9
Tested-by: syzbot+0a4c46806941297fecb9@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/69d122fd.050a0220.2dbe29.001c.GAE@google.com/
Suggested-by: Kusaram Devineni <kusaram@devineni.in>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Reviewed-by: Kees Cook <kees@kernel.org>
---
 fs/signalfd.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/fs/signalfd.c b/fs/signalfd.c
index dff53745e352..22bc0870a824 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -48,17 +48,30 @@ static int signalfd_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
+static void refine_sigmask(struct signalfd_ctx *ctx, sigset_t *sigmask)
+{
+	struct k_sigaction *k = current->sighand->action;
+	int n;
+
+	*sigmask = ctx->sigmask;
+	for (n = 1; n <= _NSIG; ++n, ++k) {
+		if (k->sa.sa_flags & SA_IMMUTABLE)
+			sigaddset(sigmask, n);
+	}
+}
+
 static __poll_t signalfd_poll(struct file *file, poll_table *wait)
 {
 	struct signalfd_ctx *ctx = file->private_data;
 	__poll_t events = 0;
+	sigset_t sigmask;
 
 	poll_wait(file, &current->sighand->signalfd_wqh, wait);
 
 	spin_lock_irq(&current->sighand->siglock);
-	if (next_signal(&current->pending, &ctx->sigmask) ||
-	    next_signal(&current->signal->shared_pending,
-			&ctx->sigmask))
+	refine_sigmask(ctx, &sigmask);
+	if (next_signal(&current->pending, &sigmask) ||
+	    next_signal(&current->signal->shared_pending, &sigmask))
 		events |= EPOLLIN;
 	spin_unlock_irq(&current->sighand->siglock);
 
@@ -155,11 +168,13 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 				int nonblock)
 {
 	enum pid_type type;
-	ssize_t ret;
 	DECLARE_WAITQUEUE(wait, current);
+	sigset_t sigmask;
+	ssize_t ret;
 
 	spin_lock_irq(&current->sighand->siglock);
-	ret = dequeue_signal(&ctx->sigmask, info, &type);
+	refine_sigmask(ctx, &sigmask);
+	ret = dequeue_signal(&sigmask, info, &type);
 	switch (ret) {
 	case 0:
 		if (!nonblock)
@@ -174,7 +189,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 	add_wait_queue(&current->sighand->signalfd_wqh, &wait);
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
-		ret = dequeue_signal(&ctx->sigmask, info, &type);
+		ret = dequeue_signal(&sigmask, info, &type);
 		if (ret != 0)
 			break;
 		if (signal_pending(current)) {
@@ -184,6 +199,7 @@ static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, kernel_siginfo_t *info
 		spin_unlock_irq(&current->sighand->siglock);
 		schedule();
 		spin_lock_irq(&current->sighand->siglock);
+		refine_sigmask(ctx, &sigmask);
 	}
 	spin_unlock_irq(&current->sighand->siglock);
 
-- 
2.52.0