linux-kernel - Re: [PATCH] x86: execve and sigreturn syscalls must return via iret

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150323075637.GA25620@gmail.com>
Date:	Mon, 23 Mar 2015 08:56:37 +0100
From:	Ingo Molnar <mingo@...nel.org>
To:	Brian Gerst <brgerst@...il.com>
Cc:	linux-kernel@...r.kernel.org, x86@...nel.org,
	Denys Vlasenko <dvlasenk@...hat.com>,
	Andy Lutomirski <luto@...capital.net>,
	Borislav Petkov <bp@...en8.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: execve and sigreturn syscalls must return via iret


* Brian Gerst <brgerst@...il.com> wrote:

> Both the execve and sigreturn family of syscalls have the ability to change
> registers in ways that may not be compatabile with the syscall path they
> were called from.  In particular, sysret and sysexit can't handle non-default
> %cs and %ss, and some bits in eflags.  These syscalls have stubs that are
> hardcoded to jump to the iret path, and not return to the original syscall
> path.  Commit 76f5df43cab5e765c0bd42289103e8f625813ae1 (Always allocate a
> complete "struct pt_regs" on the kernel stack) recently changed this for
> some 32-bit compat syscalls, but introduced a bug where execve from a 32-bit
> program to a 64-bit program would fail because it still returned via sysretl.
> This caused Wine to fail when built for both 32-bit and 64-bit.
> 
> This patch sets TIF_NOTIFY_RESUME for execve and sigreturn so that the iret
> path is always taken on exit to userspace.
> 
> Signed-off-by: Brian Gerst <brgerst@...il.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Denys Vlasenko <dvlasenk@...hat.com>
> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> ---
>  arch/x86/ia32/ia32_signal.c        | 2 ++
>  arch/x86/include/asm/ptrace.h      | 2 +-
>  arch/x86/include/asm/thread_info.h | 7 +++++++
>  arch/x86/kernel/process_32.c       | 6 +-----
>  arch/x86/kernel/process_64.c       | 1 +
>  arch/x86/kernel/signal.c           | 2 ++
>  6 files changed, 14 insertions(+), 6 deletions(-)

Applied the fix to tip:x86/asm, thanks Brian!

> +
> +/*
> + * force syscall return via iret by making it look as if there was
> + * some work pending.
> +*/
> +#define force_iret() set_thread_flag(TIF_NOTIFY_RESUME)

I extended this comment to:

/*
 * Force syscall return via IRET by making it look as if there was
 * some work pending. IRET is our most capable (but slowest) syscall
 * return path, which is able to restore modified SS, CS and certain
 * EFLAGS values that other (fast) syscall return instructions
 * are not able to restore properly.
 */
#define force_iret() set_thread_flag(TIF_NOTIFY_RESUME)

Just to preserve the underlying reason for force_iret() for the future 
and such.

Btw., it might be a worthwile optimization to detect non-standard SS, 
CS and EFLAGS values and only force_iret() in that case, that will 
speed up 99.9999% of execve() and sigreturn() syscalls and only force 
the 'weird' process startup modes into the slow return path.

>From an access security POV it should be a relatively safe 
optimization: if we get it wrong then we don't allow certain ABI 
angles, but we won't make the kernel unsafe AFAICS.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/