Re: [PATCH 17/20] signal/x86: In emulate_vsyscall force a signal instead of calling do_exit

Kees Cook
2021-10-21T16:36:47Z

On Wed, Oct 20, 2021 at 12:44:03PM -0500, Eric W. Biederman wrote:
> Directly calling do_exit with a signal number has the problem that
> all of the side effects of the signal don't happen, such as
> killing all of the threads of a process instead of just the
> calling thread.
>
> So replace do_exit(SIGSYS) with force_fatal_sig(SIGSYS) which
> causes the signal handling to take it's normal path and work
> as expected.
>
> Cc: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
> arch/x86/entry/vsyscall/vsyscall_64.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/entry/vsyscall/vsyscall_64.c b/arch/x86/entry/vsyscall/vsyscall_64.c
> index 1b40b9297083..0b6b277ee050 100644
> --- a/arch/x86/entry/vsyscall/vsyscall_64.c
> +++ b/arch/x86/entry/vsyscall/vsyscall_64.c
> @@ -226,7 +226,8 @@ bool emulate_vsyscall(unsigned long error_code,
> if ((!tmp && regs->orig_ax != syscall_nr) || regs->ip != address) {
> warn_bad_vsyscall(KERN_DEBUG, regs,
> "seccomp tried to change syscall nr or ip");
> - do_exit(SIGSYS);
> + force_fatal_sig(SIGSYS);
> + return true;
> }
> regs->orig_ax = -1;
> if (tmp)

This looks correct to me, but please double-check the x86 selftests if
you haven't already.

Reviewed-by: Kees Cook <keescook@chromium.org>

--
Kees Cook