All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Kusaram Devineni <kusaram@devineni.in>,
	Jens Axboe <axboe@kernel.dk>,
	linux-kernel@vger.kernel.org, io-uring@vger.kernel.org,
	Christian Brauner <brauner@kernel.org>
Subject: Re: [PATCH] signalfd: don't dequeue the forced fatal signals
Date: Sun, 5 Apr 2026 21:39:31 -0700	[thread overview]
Message-ID: <202604052136.440E9CFA44@keescook> (raw)
In-Reply-To: <adKJMRkQJXEwHs-j@redhat.com>

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

  reply	other threads:[~2026-04-06  4:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-05 16:09 [PATCH] signalfd: don't dequeue the forced fatal signals Oleg Nesterov
2026-04-06  4:39 ` Kees Cook [this message]
2026-04-06 13:32   ` Oleg Nesterov
2026-04-06 13:37   ` [PATCH v2] " Oleg Nesterov
2026-04-07 20:10 ` [PATCH] " kernel test robot
2026-04-07 21:22   ` Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=202604052136.440E9CFA44@keescook \
    --to=kees@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=io-uring@vger.kernel.org \
    --cc=kusaram@devineni.in \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.