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.