[PATCH] x86/asm/entry/64: Fix comment about SYSENTER MSRs

[PATCH] x86/asm/entry/64: Fix comment about SYSENTER MSRs

[Denys Vlasenko]

The comment is ancient, it dates to the time when only AMD's
x86_64 implementation existed. AMD wasn't (and still isn't)
supporting SYSENTER, so these writes were "just in case" back then.

This has changed: Intel's x86_64 appeared, and Indel does support
SYSENTER in long mode. "Some future 64-bit CPU" is here already.

The code may appear "buggy" for AMD as it stands, since MSR_IA32_SYSENTER_EIP
is only 32-bit for AMD CPUs. Writing a kernel function's address to it
would drop high bits. Subsequent use of this MSR for branch via SYSENTER
seem to allow user to transition to CPL0 while executing his code. Scary, eh?

Explain why that is not a bug: because SYSENTER insn would not work
on AMD CPU.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
CC: Linus Torvalds <torvalds@linux-foundation.org>
CC: Steven Rostedt <rostedt@goodmis.org>
CC: Ingo Molnar <mingo@kernel.org>
CC: Borislav Petkov <bp@alien8.de>
CC: "H. Peter Anvin" <hpa@zytor.com>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Frederic Weisbecker <fweisbec@gmail.com>
CC: Alexei Starovoitov <ast@plumgrid.com>
CC: Will Drewry <wad@chromium.org>
CC: Kees Cook <keescook@chromium.org>
CC: x86@kernel.org
CC: linux-kernel@vger.kernel.org
---
 arch/x86/kernel/cpu/common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b5ad0fa..7c57b12 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1212,8 +1212,10 @@ void syscall_init(void)
 #ifdef CONFIG_IA32_EMULATION
 	wrmsrl(MSR_CSTAR, ia32_cstar_target);
 	/*
-	 * Always load these, in case some future 64-bit CPU supports
-	 * SYSENTER from compat mode too:
+	 * This only works for Intel CPUs.
+	 * On AMD, these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
+	 * This does not cause SYSENTER to jump to wrong location because
+	 * AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit).
 	 */
 	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
-- 
1.8.1.4

[tip:x86/asm] x86/asm/entry/64: Fix comment about SYSENTER MSRs

[tip-bot for Denys Vlasenko]

Commit-ID:  487d1edb9a6005cf790c7fe59f25ad1e5cb5817b
Gitweb:     http://git.kernel.org/tip/487d1edb9a6005cf790c7fe59f25ad1e5cb5817b
Author:     Denys Vlasenko <dvlasenk@redhat.com>
AuthorDate: Fri, 27 Mar 2015 11:59:16 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 27 Mar 2015 12:23:16 +0100

x86/asm/entry/64: Fix comment about SYSENTER MSRs

The comment is ancient, it dates to the time when only AMD's
x86_64 implementation existed. AMD wasn't (and still isn't)
supporting SYSENTER, so these writes were "just in case" back
then.

This has changed: Intel's x86_64 appeared, and Intel does
support SYSENTER in long mode. "Some future 64-bit CPU" is here
already.

The code may appear "buggy" for AMD as it stands, since
MSR_IA32_SYSENTER_EIP is only 32-bit for AMD CPUs. Writing a
kernel function's address to it would drop high bits. Subsequent
use of this MSR for branch via SYSENTER seem to allow user to
transition to CPL0 while executing his code. Scary, eh?

Explain why that is not a bug: because SYSENTER insn would not
work on AMD CPU.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/1427453956-21931-1-git-send-email-dvlasenk@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/common.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index c928a7a..71e4adc 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1173,8 +1173,10 @@ void syscall_init(void)
 #ifdef CONFIG_IA32_EMULATION
 	wrmsrl(MSR_CSTAR, ia32_cstar_target);
 	/*
-	 * Always load these, in case some future 64-bit CPU supports
-	 * SYSENTER from compat mode too:
+	 * This only works on Intel CPUs.
+	 * On AMD CPUs these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
+	 * This does not cause SYSENTER to jump to the wrong location, because
+	 * AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit).
 	 */
 	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
 	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);