1. Patchew
  2. linux
  3. View series

[PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks

Oleg Nesterov posted 1 patch 2 weeks, 6 days ago 
kernel/signal.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks
Posted by Oleg Nesterov 2 weeks, 6 days ago 
Now that kernel_clone() checks valid_signal(args->exit_signal), the "sig"
argument of do_notify_parent() must always be valid or we have a bug.

However, do_notify_parent() only checks that sig != -1 at the start, then
it does another valid_signal() check before __send_signal_locked().

This is confusing. Change do_notify_parent() to WARN and return early if
valid_signal(sig) is false.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/signal.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 86aad7badb9a..683ef92f7234 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2171,7 +2171,8 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
 	bool autoreap = false;
 	u64 utime, stime;
 
-	WARN_ON_ONCE(sig == -1);
+	if (WARN_ON_ONCE(!valid_signal(sig)))
+		return false;
 
 	/* do_notify_parent_cldstop should have been called instead.  */
 	WARN_ON_ONCE(task_is_stopped_or_traced(tsk));
@@ -2252,7 +2253,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
 	 * Send with __send_signal as si_pid and si_uid are in the
 	 * parent's namespaces.
 	 */
-	if (valid_signal(sig) && sig)
+	if (sig)
 		__send_signal_locked(sig, &info, tsk->parent, PIDTYPE_TGID, false);
 	__wake_up_parent(tsk, tsk->parent);
 	spin_unlock_irqrestore(&psig->siglock, flags);
-- 
2.52.0
Re: [PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks
Posted by Andrew Morton 2 weeks, 6 days ago 
On Tue, 17 Mar 2026 14:58:18 +0100 Oleg Nesterov <oleg@redhat.com> wrote:

> Now that kernel_clone() checks valid_signal(args->exit_signal), the "sig"
> argument of do_notify_parent() must always be valid or we have a bug.
> 
> However, do_notify_parent() only checks that sig != -1 at the start, then
> it does another valid_signal() check before __send_signal_locked().
> 
> This is confusing. Change do_notify_parent() to WARN and return early if
> valid_signal(sig) is false.

Sashiko has a question:
	https://sashiko.dev/#/patchset/abld-ilvMEZ7VgMw%40redhat.com
Re: [PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks
Posted by Oleg Nesterov 2 weeks, 6 days ago 
On 03/17, Andrew Morton wrote:
>
> On Tue, 17 Mar 2026 14:58:18 +0100 Oleg Nesterov <oleg@redhat.com> wrote:
>
> > Now that kernel_clone() checks valid_signal(args->exit_signal), the "sig"
> > argument of do_notify_parent() must always be valid or we have a bug.
> >
> > However, do_notify_parent() only checks that sig != -1 at the start, then
> > it does another valid_signal() check before __send_signal_locked().
> >
> > This is confusing. Change do_notify_parent() to WARN and return early if
> > valid_signal(sig) is false.
>
> Sashiko has a question:
> 	https://sashiko.dev/#/patchset/abld-ilvMEZ7VgMw%40redhat.com

I think that userpace can't bypass kernel_clone() (which checks valid_signal)
before copy_process().

This includes ia32_clone() and sparc_clone() mentioned in the link above.

There are in-kernel users (fork_idle, create_io_thread, vhost_task_create).
But if they pass a non-valid exit_signal (they don't), we do have a kernel
bug and WARN_ON() added by this patch should catch the problem.

In short. From the link above:

	While kernel_clone() expects the caller to validate args->exit_signal

this was true before

	kernel-fork-validate-exit_signal-in-kernel_clone.patch

from Deepanshu, and my cleanup depends on it.

Oleg.
Re: [PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks
Posted by Deepanshu Kartikey 2 weeks, 6 days ago 
On Tue, Mar 17, 2026 at 7:28 PM Oleg Nesterov <oleg@redhat.com> wrote:
>
> Now that kernel_clone() checks valid_signal(args->exit_signal), the "sig"
> argument of do_notify_parent() must always be valid or we have a bug.
>
> However, do_notify_parent() only checks that sig != -1 at the start, then
> it does another valid_signal() check before __send_signal_locked().
>
> This is confusing. Change do_notify_parent() to WARN and return early if
> valid_signal(sig) is false.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  kernel/signal.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 86aad7badb9a..683ef92f7234 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2171,7 +2171,8 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
>         bool autoreap = false;
>         u64 utime, stime;
>
> -       WARN_ON_ONCE(sig == -1);
> +       if (WARN_ON_ONCE(!valid_signal(sig)))
> +               return false;
>
>         /* do_notify_parent_cldstop should have been called instead.  */
>         WARN_ON_ONCE(task_is_stopped_or_traced(tsk));
> @@ -2252,7 +2253,7 @@ bool do_notify_parent(struct task_struct *tsk, int sig)
>          * Send with __send_signal as si_pid and si_uid are in the
>          * parent's namespaces.
>          */
> -       if (valid_signal(sig) && sig)
> +       if (sig)
>                 __send_signal_locked(sig, &info, tsk->parent, PIDTYPE_TGID, false);
>         __wake_up_parent(tsk, tsk->parent);
>         spin_unlock_irqrestore(&psig->siglock, flags);
> --
> 2.52.0
>
>


Acked-by: Deepanshu Kartikey <Kartikey406@gmail.com>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.