1. Patchew
  2. linux
  3. View series

[PATCH v3] kernel/fork: validate exit_signal in kernel_clone()

Deepanshu Kartikey posted 1 patch 3 weeks ago 
kernel/fork.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
[PATCH v3] kernel/fork: validate exit_signal in kernel_clone()
Posted by Deepanshu Kartikey 3 weeks ago 
When a child process exits, it sends exit_signal to its parent via
do_notify_parent(). The clone() syscall constructs exit_signal as:

(lower_32_bits(clone_flags) & CSIGNAL)

CSIGNAL is 0xff, so values in the range 65-255 are possible. However,
valid_signal() only accepts signals up to _NSIG (64 on x86_64). A
non-zero non-valid exit_signal acts the same as exit_signal == 0:
the parent process is not signaled when the child terminates.

The syzkaller reproducer triggers this by calling clone() with
flags=0x80, resulting in exit_signal = (0x80 & CSIGNAL) = 128, which
exceeds _NSIG and is not a valid signal.

The v1 of this patch added the check only in the clone() syscall
handler, which is incomplete. kernel_clone() has other callers such
as sys_ia32_clone() which would remain unprotected. Move the check
to kernel_clone() to cover all callers.

Since the valid_signal() check is now in kernel_clone() and covers
all callers including clone3(), the same check in
copy_clone_args_from_user() becomes redundant and is removed. The
higher 32bits check for clone3() is kept as it is clone3() specific.

Note that this is a user-visible change: previously, passing an invalid
exit_signal to clone() was silently accepted. The man page for clone()
does not document any defined behavior for invalid exit_signal values,
so rejecting them with -EINVAL is the correct behavior. It is unlikely
that any sane application relies on passing an invalid exit_signal.

Reported-by: syzbot+bbe6b99feefc3a0842de@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bbe6b99feefc3a0842de
Tested-by: syzbot+bbe6b99feefc3a0842de@syzkaller.appspotmail.com
Link: https://lore.kernel.org/all/20260307064202.353405-1-kartikey406@gmail.com/T/ [v1]
Link: https://lore.kernel.org/all/20260316104536.558108-1-kartikey406@gmail.com/T/ [v2]
Fixes: 3f2c788a1314 ("fork: prevent accidental access to clone3 features")
Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
---
v3:
  - Remove now redundant valid_signal() check from
    copy_clone_args_from_user() since kernel_clone() now covers
    all callers including clone3()
  - Update changelog to remove WARN_ON mention and describe
    actual bug behavior instead

v2:
  - Move valid_signal() check from clone() syscall handler to
    kernel_clone() to cover all callers including sys_ia32_clone()
  - Update changelog to document user-visible behavior change
---
 kernel/fork.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/kernel/fork.c b/kernel/fork.c
index 947a8dbce06a..73f7fe958a30 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2687,6 +2687,9 @@ pid_t kernel_clone(struct kernel_clone_args *args)
 	    (args->pidfd == args->parent_tid))
 		return -EINVAL;
 
+	if (!valid_signal(args->exit_signal))
+		return -EINVAL;
+
 	/*
 	 * Determine whether and which event to report to ptracer.  When
 	 * called from kernel_thread or CLONE_UNTRACED is explicitly
@@ -2885,11 +2888,9 @@ static noinline int copy_clone_args_from_user(struct kernel_clone_args *kargs,
 		return -EINVAL;
 
 	/*
-	 * Verify that higher 32bits of exit_signal are unset and that
-	 * it is a valid signal
+	 * Verify that higher 32bits of exit_signal are unset
 	 */
-	if (unlikely((args.exit_signal & ~((u64)CSIGNAL)) ||
-		     !valid_signal(args.exit_signal)))
+	if (unlikely(args.exit_signal & ~((u64)CSIGNAL)))
 		return -EINVAL;
 
 	if ((args.flags & CLONE_INTO_CGROUP) &&
-- 
2.43.0
Re: [PATCH v3] kernel/fork: validate exit_signal in kernel_clone()
Posted by Oleg Nesterov 2 weeks, 6 days ago 
On 03/16, Deepanshu Kartikey wrote:
>
> Note that this is a user-visible change: previously, passing an invalid
> exit_signal to clone() was silently accepted. The man page for clone()
> does not document any defined behavior for invalid exit_signal values,
> so rejecting them with -EINVAL is the correct behavior. It is unlikely
> that any sane application relies on passing an invalid exit_signal.

Agreed...

> -	if (unlikely((args.exit_signal & ~((u64)CSIGNAL)) ||
> -		     !valid_signal(args.exit_signal)))
> +	if (unlikely(args.exit_signal & ~((u64)CSIGNAL)))
>  		return -EINVAL;

I guess this can die too, but OK, this needs another patch/changelog.

Acked-by: Oleg Nesterov <oleg@redhat.com>


Andrew, I will slightly update the changelog and resend
do_notify_parent-sanitize-the-valid_signal-checks.patch
in reply to this patch from Deepanshu.

Oleg.
Re: [PATCH v3] kernel/fork: validate exit_signal in kernel_clone()
Posted by Christian Brauner 2 weeks, 6 days ago 
On Tue, Mar 17, 2026 at 01:48:42PM +0100, Oleg Nesterov wrote:
> On 03/16, Deepanshu Kartikey wrote:
> >
> > Note that this is a user-visible change: previously, passing an invalid
> > exit_signal to clone() was silently accepted. The man page for clone()
> > does not document any defined behavior for invalid exit_signal values,
> > so rejecting them with -EINVAL is the correct behavior. It is unlikely
> > that any sane application relies on passing an invalid exit_signal.
> 
> Agreed...
> 
> > -	if (unlikely((args.exit_signal & ~((u64)CSIGNAL)) ||
> > -		     !valid_signal(args.exit_signal)))
> > +	if (unlikely(args.exit_signal & ~((u64)CSIGNAL)))
> >  		return -EINVAL;
> 
> I guess this can die too, but OK, this needs another patch/changelog.
> 
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> 
> 
> Andrew, I will slightly update the changelog and resend
> do_notify_parent-sanitize-the-valid_signal-checks.patch
> in reply to this patch from Deepanshu.

I'll take this via the pidfs/kernel tree. I also have a bunch of work
touching this stuff queued there.
Re: [PATCH v3] kernel/fork: validate exit_signal in kernel_clone()
Posted by Oleg Nesterov 2 weeks, 6 days ago 
On 03/17, Christian Brauner wrote:
>
> > Andrew, I will slightly update the changelog and resend
> > do_notify_parent-sanitize-the-valid_signal-checks.patch
> > in reply to this patch from Deepanshu.
>
> I'll take this via the pidfs/kernel tree. I also have a bunch of work
> touching this stuff queued there.

Ah... Then can you also take

	[PATCH -mm v2] do_notify_parent: sanitize the valid_signal() checks
	https://lore.kernel.org/all/abld-ilvMEZ7VgMw@redhat.com/

which I sent a minute ago on top of this patch? (I have trimmed the huge CC,
so you wasn't CC'ed).

Cough... and may be another/unrelated

	[PATCH] pidfd_prepare: don't pass O_RDWR to pidfs_alloc_file()
	https://lore.kernel.org/all/aZ8gwQYpIjVktcOs@redhat.com/

which I sent you almost a month ago?

Oleg.
[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.