linux-kernel - [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-next>] [day] [month] [year] [list]

Date:	Sat, 21 Mar 2015 18:54:21 -0400
From:	Brian Gerst <brgerst@...il.com>
To:	linux-kernel@...r.kernel.org
Cc:	x86@...nel.org, Ingo Molnar <mingo@...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: [PATCH] x86: execve and sigreturn syscalls must return via iret

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(-)

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index d0165c9..1f5e2b0 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -203,6 +203,8 @@ static int ia32_restore_sigcontext(struct pt_regs *regs,
 
 	err |= restore_xstate_sig(buf, 1);
 
+	force_iret();
+
 	return err;
 }
 
diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h
index 74bb2e0..83b874d 100644
--- a/arch/x86/include/asm/ptrace.h
+++ b/arch/x86/include/asm/ptrace.h
@@ -251,7 +251,7 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
  */
 #define arch_ptrace_stop_needed(code, info)				\
 ({									\
-	set_thread_flag(TIF_NOTIFY_RESUME);				\
+	force_iret();							\
 	false;								\
 })
 
diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
index ba115eb..861c1dd 100644
--- a/arch/x86/include/asm/thread_info.h
+++ b/arch/x86/include/asm/thread_info.h
@@ -260,6 +260,13 @@ static inline bool is_ia32_task(void)
 #endif
 	return false;
 }
+
+/*
+ * 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)
+
 #endif	/* !__ASSEMBLY__ */
 
 #ifndef __ASSEMBLY__
diff --git a/arch/x86/kernel/process_32.c b/arch/x86/kernel/process_32.c
index 1b9963f..26c596d 100644
--- a/arch/x86/kernel/process_32.c
+++ b/arch/x86/kernel/process_32.c
@@ -206,11 +206,7 @@ start_thread(struct pt_regs *regs, unsigned long new_ip, unsigned long new_sp)
 	regs->ip		= new_ip;
 	regs->sp		= new_sp;
 	regs->flags		= X86_EFLAGS_IF;
-	/*
-	 * force it to the iret return path by making it look as if there was
-	 * some work pending.
-	 */
-	set_thread_flag(TIF_NOTIFY_RESUME);
+	force_iret();
 }
 EXPORT_SYMBOL_GPL(start_thread);
 
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 97f5658..da8b745 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -239,6 +239,7 @@ start_thread_common(struct pt_regs *regs, unsigned long new_ip,
 	regs->cs		= _cs;
 	regs->ss		= _ss;
 	regs->flags		= X86_EFLAGS_IF;
+	force_iret();
 }
 
 void
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index edcb862..eaa2c5e 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -108,6 +108,8 @@ int restore_sigcontext(struct pt_regs *regs, struct sigcontext __user *sc,
 
 	err |= restore_xstate_sig(buf, config_enabled(CONFIG_X86_32));
 
+	force_iret();
+
 	return err;
 }
 
-- 
2.1.0

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