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

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

[Deepanshu Kartikey]

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), causing
a WARN_ON in do_notify_parent() when the process exits:

  WARNING: kernel/signal.c:2174 do_notify_parent+0xc7e/0xd70

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.

clone3() already validates exit_signal in copy_clone_args_from_user().
The comment above kernel_clone() states that callers are expected to
validate exit_signal, but several callers never did. Adding the check
to kernel_clone() enforces this for all callers centrally.

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]
Fixes: 3f2c788a1314 ("fork: prevent accidental access to clone3 features")
Signed-off-by: Deepanshu Kartikey <Kartikey406@gmail.com>
---
 kernel/fork.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/fork.c b/kernel/fork.c
index 947a8dbce06a..89d7eb67baf5 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2687,6 +2687,8 @@ 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
-- 
2.43.0

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

[Oleg Nesterov]

Deepanshu,

Let me repeat, the changelog should be updated.

On 03/16, Deepanshu Kartikey wrote:
>
> 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), causing
> a WARN_ON in do_notify_parent() when the process exits:
>
>   WARNING: kernel/signal.c:2174 do_notify_parent+0xc7e/0xd70

Again, do_notify_parent-sanitize-the-valid_signal-checks.patch
was dropped. do_notify_parent() won't WARN() in this case.

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

Yes, it only documents that if exit_signal == 0 then the parent process
is not signaled when the child terminates. But in fact a non-zero non-valid
signal acts the same way.

> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -2687,6 +2687,8 @@ pid_t kernel_clone(struct kernel_clone_args *args)
>  	    (args->pidfd == args->parent_tid))
>  		return -EINVAL;
>  
> +	if (!valid_signal(args->exit_signal))
> +		return -EINVAL;

OK, but then it also makes sense to remove the same valid_signal() check
in copy_clone_args_from_user() ?

Oleg.