[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

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

[kernel test robot]

Hi Oleg,

kernel test robot noticed the following build warnings:

[auto build test WARNING on brauner-vfs/vfs.all]
[also build test WARNING on akpm-mm/mm-everything kees/for-next/pstore kees/for-next/kspp linus/master v7.0-rc7 next-20260406]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Oleg-Nesterov/signalfd-don-t-dequeue-the-forced-fatal-signals/20260407-131556
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link:    https://lore.kernel.org/r/adKJMRkQJXEwHs-j%40redhat.com
patch subject: [PATCH] signalfd: don't dequeue the forced fatal signals
config: x86_64-randconfig-123-20260407 (https://download.01.org/0day-ci/archive/20260408/202604080450.mkKRp9Mk-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
sparse: v0.6.5-rc1
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260408/202604080450.mkKRp9Mk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604080450.mkKRp9Mk-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> fs/signalfd.c:53:40: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct k_sigaction *k @@     got struct k_sigaction [noderef] __rcu * @@
   fs/signalfd.c:53:40: sparse:     expected struct k_sigaction *k
   fs/signalfd.c:53:40: sparse:     got struct k_sigaction [noderef] __rcu *
   fs/signalfd.c:69:33: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct wait_queue_head [usertype] *wait_address @@     got struct wait_queue_head [noderef] __rcu * @@
   fs/signalfd.c:69:33: sparse:     expected struct wait_queue_head [usertype] *wait_address
   fs/signalfd.c:69:33: sparse:     got struct wait_queue_head [noderef] __rcu *
   fs/signalfd.c:71:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:71:31: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:71:31: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:76:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:76:33: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:76:33: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:110:32: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:128:33: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:131:33: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:135:33: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:151:32: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:155:38: sparse: sparse: cast removes address space '__user' of expression
   fs/signalfd.c:175:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:175:31: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:175:31: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:185:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:185:41: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:185:41: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:189:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct wait_queue_head *wq_head @@     got struct wait_queue_head [noderef] __rcu * @@
   fs/signalfd.c:189:32: sparse:     expected struct wait_queue_head *wq_head
   fs/signalfd.c:189:32: sparse:     got struct wait_queue_head [noderef] __rcu *
   fs/signalfd.c:199:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:199:41: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:199:41: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:201:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:201:39: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:201:39: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:204:33: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:204:33: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:204:33: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:206:35: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct wait_queue_head *wq_head @@     got struct wait_queue_head [noderef] __rcu * @@
   fs/signalfd.c:206:35: sparse:     expected struct wait_queue_head *wq_head
   fs/signalfd.c:206:35: sparse:     got struct wait_queue_head [noderef] __rcu *
   fs/signalfd.c:305:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:305:39: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:305:39: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:307:41: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct spinlock [usertype] *lock @@     got struct spinlock [noderef] __rcu * @@
   fs/signalfd.c:307:41: sparse:     expected struct spinlock [usertype] *lock
   fs/signalfd.c:307:41: sparse:     got struct spinlock [noderef] __rcu *
   fs/signalfd.c:309:17: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct wait_queue_head *wq_head @@     got struct wait_queue_head [noderef] __rcu * @@
   fs/signalfd.c:309:17: sparse:     expected struct wait_queue_head *wq_head
   fs/signalfd.c:309:17: sparse:     got struct wait_queue_head [noderef] __rcu *

vim +53 fs/signalfd.c

    50	
    51	static void mk_sigmask(struct signalfd_ctx *ctx, sigset_t *sigmask)
    52	{
  > 53		struct k_sigaction *k = current->sighand->action;
    54		int n;
    55	
    56		*sigmask = ctx->sigmask;
    57		for (n = 1; n <= _NSIG; ++n, ++k) {
    58			if (k->sa.sa_flags & SA_IMMUTABLE)
    59				sigaddset(sigmask, n);
    60		}
    61	}
    62	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Oleg Nesterov]

On 04/08, kernel test robot wrote:
>
> kernel test robot noticed the following build warnings:

...

> sparse warnings: (new ones prefixed by >>)
> >> fs/signalfd.c:53:40: sparse: sparse: incorrect type in initializer (different address spaces) @@     expected struct k_sigaction *k @@     got struct k_sigaction [noderef] __rcu * @@

...

> vim +53 fs/signalfd.c
>
>     50	
>     51	static void mk_sigmask(struct signalfd_ctx *ctx, sigset_t *sigmask)
>     52	{
>   > 53		struct k_sigaction *k = current->sighand->action;

I am going to ignore this new warning...

Yes, task_struct->sighand is __rcu. Not sure this annotation makes a lot of sense.

In any case. current->sighand is always stable. Plus task->sighand is stable under siglock.

We have a lot of (correct) non-rcu deferences of ->sighand.

I think that only lock_task_sighand() needs rcu_dereference(tsk->sighand).

Say, __exit_signal() does

	sighand = rcu_dereference_check(tsk->sighand,
					lockdep_tasklist_lock_is_held());

To me this just adds the unnecessary noise. I do not want to add another precedent.

Oleg.