linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86, selftests: Add a test for the "sysret_ss_attrs" bug
@ 2015-04-24 22:09 Andy Lutomirski
  2015-04-25 13:55 ` Denys Vlasenko
  2015-05-08 13:30 ` [tip:x86/asm] x86, selftests: Add a test for the "sysret_ss_attrs " bug tip-bot for Andy Lutomirski
  0 siblings, 2 replies; 3+ messages in thread
From: Andy Lutomirski @ 2015-04-24 22:09 UTC (permalink / raw)
  To: X86 ML
  Cc: Denys Vlasenko, H. Peter Anvin, Borislav Petkov, Denys Vlasenko,
	Linus Torvalds, Brian Gerst, Ingo Molnar, Steven Rostedt,
	Oleg Nesterov, Frederic Weisbecker, Alexei Starovoitov,
	Will Drewry, Kees Cook, Linux Kernel Mailing List,
	Andy Lutomirski

On AMD CPUs, SYSRET can return with a valid SS descriptor with with
the hidden attributes set to an unusable state.  Make sure the kernel
doesn't let this happen.  This detects an as-yet-unfixed regression.

Note that the 64-bit version of this test fails on AMD CPUs on all
kernel versions, although the issue in the 64-bit case is much less
severe than in the 32-bit case.

Tests: e7d6eefaaa44 x86/vdso32/syscall.S: Do not load __USER32_DS to %ss
Reported-by: Brian Gerst <brgerst@gmail.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---

[resend -- it seems like only some recipients got it last time]

We're still mulling over the fix, but we have a good test case now.

tools/testing/selftests/x86/Makefile          |   5 +-
 tools/testing/selftests/x86/run_x86_tests.sh  |   2 +
 tools/testing/selftests/x86/sysret_ss_attrs.c | 112 ++++++++++++++++++++++++++
 tools/testing/selftests/x86/thunks.S          |  67 +++++++++++++++
 4 files changed, 185 insertions(+), 1 deletion(-)
 mode change 100644 => 100755 tools/testing/selftests/x86/run_x86_tests.sh
 create mode 100644 tools/testing/selftests/x86/sysret_ss_attrs.c
 create mode 100644 tools/testing/selftests/x86/thunks.S

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index ddf63569df5a..9309097f58e8 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -1,6 +1,6 @@
 .PHONY: all all_32 all_64 check_build32 clean run_tests
 
-TARGETS_C_BOTHBITS := sigreturn single_step_syscall
+TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
 
 BINARIES_32 := $(TARGETS_C_BOTHBITS:%=%_32)
 BINARIES_64 := $(TARGETS_C_BOTHBITS:%=%_64)
@@ -46,3 +46,6 @@ check_build32:
 	  echo "  yum install glibc-devel.*i686";			\
 	  exit 1;							\
 	fi
+
+# Some tests have additional dependencies.
+sysret_ss_attrs_64: thunks.S
diff --git a/tools/testing/selftests/x86/run_x86_tests.sh b/tools/testing/selftests/x86/run_x86_tests.sh
old mode 100644
new mode 100755
index 3fc19b376812..d25034280dd5
--- a/tools/testing/selftests/x86/run_x86_tests.sh
+++ b/tools/testing/selftests/x86/run_x86_tests.sh
@@ -4,10 +4,12 @@
 # script here.
 ./sigreturn_32 || exit 1
 ./single_step_syscall_32 || exit 1
+./sysret_ss_attrs_32 || exit 1
 
 if [[ "$uname -p" -eq "x86_64" ]]; then
     ./sigreturn_64 || exit 1
     ./single_step_syscall_64 || exit 1
+    ./sysret_ss_attrs_64 || exit 1
 fi
 
 exit 0
diff --git a/tools/testing/selftests/x86/sysret_ss_attrs.c b/tools/testing/selftests/x86/sysret_ss_attrs.c
new file mode 100644
index 000000000000..ce42d5a64009
--- /dev/null
+++ b/tools/testing/selftests/x86/sysret_ss_attrs.c
@@ -0,0 +1,112 @@
+/*
+ * sysret_ss_attrs.c - test that syscalls return valid hidden SS attributes
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * On AMD CPUs, SYSRET can return with a valid SS descriptor with with
+ * the hidden attributes set to an unusable state.  Make sure the kernel
+ * doesn't let this happen.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <err.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <pthread.h>
+
+static void *threadproc(void *ctx)
+{
+	/*
+	 * Do our best to cause sleeps on this CPU to exit the kernel and
+	 * re-enter with SS = 0.
+	 */
+	while (true)
+		;
+
+	return NULL;
+}
+
+#ifdef __x86_64__
+extern unsigned long call32_from_64(void *stack, void (*function)(void));
+
+asm (".pushsection .text\n\t"
+     ".code32\n\t"
+     "test_ss:\n\t"
+     "pushl $0\n\t"
+     "popl %eax\n\t"
+     "ret\n\t"
+     ".code64");
+extern void test_ss(void);
+#endif
+
+int main()
+{
+	/*
+	 * Start a busy-looping thread on the same CPU we're on.
+	 * For simplicity, just stick everything to CPU 0.  This will
+	 * fail in some containers, but that's probably okay.
+	 */
+	cpu_set_t cpuset;
+	CPU_ZERO(&cpuset);
+	CPU_SET(0, &cpuset);
+	if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+		printf("[WARN]\tsched_setaffinity failed\n");
+
+	pthread_t thread;
+	if (pthread_create(&thread, 0, threadproc, 0) != 0)
+		err(1, "pthread_create");
+
+#ifdef __x86_64__
+	unsigned char *stack32 = mmap(NULL, 4096, PROT_READ | PROT_WRITE,
+				      MAP_32BIT | MAP_ANONYMOUS | MAP_PRIVATE,
+				      -1, 0);
+	if (stack32 == MAP_FAILED)
+		err(1, "mmap");
+#endif
+
+	printf("[RUN]\tSyscalls followed by SS validation\n");
+
+	for (int i = 0; i < 1000; i++) {
+		/*
+		 * Go to sleep and return using sysret (if we're 64-bit
+		 * or we're 32-bit on AMD on a 64-bit kernel).  On AMD CPUs,
+		 * SYSRET doesn't fix up the cached SS descriptor, so the
+		 * kernel needs some kind of workaround to make sure that we
+		 * end the system call with a valid stack segment.  This
+		 * can be a confusing failure because the SS *selector*
+		 * is the same regardless.
+		 */
+		usleep(2);
+
+#ifdef __x86_64__
+		/*
+		 * On 32-bit, just doing a syscall through glibc is enough
+		 * to cause a crash if our cached SS descriptor is invalid.
+		 * On 64-bit, it's not, so try extra hard.
+		 */
+		call32_from_64(stack32 + 4088, test_ss);
+#endif
+	}
+
+	printf("[OK]\tWe survived\n");
+
+#ifdef __x86_64__
+	munmap(stack32, 4096);
+#endif
+
+	return 0;
+}
diff --git a/tools/testing/selftests/x86/thunks.S b/tools/testing/selftests/x86/thunks.S
new file mode 100644
index 000000000000..ce8a995bbb17
--- /dev/null
+++ b/tools/testing/selftests/x86/thunks.S
@@ -0,0 +1,67 @@
+/*
+ * thunks.S - assembly helpers for mixed-bitness code
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * These are little helpers that make it easier to switch bitness on
+ * the fly.
+ */
+
+	.text
+
+	.global call32_from_64
+	.type call32_from_64, @function
+call32_from_64:
+	// rdi: stack to use
+	// esi: function to call
+
+	// Save registers
+	pushq %rbx
+	pushq %rbp
+	pushq %r12
+	pushq %r13
+	pushq %r14
+	pushq %r15
+	pushfq
+
+	// Switch stacks
+	mov %rsp,(%rdi)
+	mov %rdi,%rsp
+
+	// Switch to compatibility mode
+	pushq $0x23  /* USER32_CS */
+	pushq $1f
+	lretq
+
+1:
+	.code32
+	// Call the function
+	call *%esi
+	// Switch back to long mode
+	jmp $0x33,$1f
+	.code64
+
+1:
+	// Restore the stack
+	mov (%rsp),%rsp
+
+	// Restore registers
+	popfq
+	popq %r15
+	popq %r14
+	popq %r13
+	popq %r12
+	popq %rbp
+	popq %rbx
+
+	ret
+
+.size call32_from_64, .-call32_from_64
-- 
2.3.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86, selftests: Add a test for the "sysret_ss_attrs" bug
  2015-04-24 22:09 [PATCH] x86, selftests: Add a test for the "sysret_ss_attrs" bug Andy Lutomirski
@ 2015-04-25 13:55 ` Denys Vlasenko
  2015-05-08 13:30 ` [tip:x86/asm] x86, selftests: Add a test for the "sysret_ss_attrs " bug tip-bot for Andy Lutomirski
  1 sibling, 0 replies; 3+ messages in thread
From: Denys Vlasenko @ 2015-04-25 13:55 UTC (permalink / raw)
  To: Andy Lutomirski, X86 ML
  Cc: H. Peter Anvin, Borislav Petkov, Denys Vlasenko, Linus Torvalds,
	Brian Gerst, Ingo Molnar, Steven Rostedt, Oleg Nesterov,
	Frederic Weisbecker, Alexei Starovoitov, Will Drewry, Kees Cook,
	Linux Kernel Mailing List

On 04/25/2015 12:09 AM, Andy Lutomirski wrote:
> On AMD CPUs, SYSRET can return with a valid SS descriptor with with
> the hidden attributes set to an unusable state.  Make sure the kernel
> doesn't let this happen.  This detects an as-yet-unfixed regression.
> 
> Note that the 64-bit version of this test fails on AMD CPUs on all
> kernel versions, although the issue in the 64-bit case is much less
> severe than in the 32-bit case.

Confirmed to detect the "bug" in 64-bit userspace.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [tip:x86/asm] x86, selftests: Add a test for the "sysret_ss_attrs " bug
  2015-04-24 22:09 [PATCH] x86, selftests: Add a test for the "sysret_ss_attrs" bug Andy Lutomirski
  2015-04-25 13:55 ` Denys Vlasenko
@ 2015-05-08 13:30 ` tip-bot for Andy Lutomirski
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot for Andy Lutomirski @ 2015-05-08 13:30 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: keescook, akpm, vda.linux, luto, tglx, fweisbec, ast, mingo, oleg,
	rostedt, bp, linux-kernel, peterz, torvalds, hpa, dvlasenk,
	brgerst, wad

Commit-ID:  e22438f8e997ac1c8911d8808b6a4c492cd8bc6e
Gitweb:     http://git.kernel.org/tip/e22438f8e997ac1c8911d8808b6a4c492cd8bc6e
Author:     Andy Lutomirski <luto@kernel.org>
AuthorDate: Fri, 24 Apr 2015 15:09:19 -0700
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 8 May 2015 13:33:59 +0200

x86, selftests: Add a test for the "sysret_ss_attrs" bug

On AMD CPUs, SYSRET can return with a valid SS descriptor with
with the hidden attributes set to an unusable state.  Make sure
the kernel doesn't let this happen.  This detects an
as-yet-unfixed regression.

Note that the 64-bit version of this test fails on AMD CPUs on
all kernel versions, although the issue in the 64-bit case is
much less severe than in the 32-bit case.

Reported-by: Brian Gerst <brgerst@gmail.com>
Tested-by: Denys Vlasenko <dvlasenk@redhat.com>
Signed-off-by: Andy Lutomirski <luto@kernel.org>
Tests: e7d6eefaaa44 ("x86/vdso32/syscall.S: Do not load __USER32_DS to %ss")
Cc: Alexei Starovoitov <ast@plumgrid.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Denys Vlasenko <vda.linux@googlemail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Will Drewry <wad@chromium.org>
Link: http://lkml.kernel.org/r/resend_4d740841bac383742949e2fefb03982736595087.git.luto@kernel.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/testing/selftests/x86/Makefile          |   5 +-
 tools/testing/selftests/x86/run_x86_tests.sh  |   2 +
 tools/testing/selftests/x86/sysret_ss_attrs.c | 112 ++++++++++++++++++++++++++
 tools/testing/selftests/x86/thunks.S          |  67 +++++++++++++++
 4 files changed, 185 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index ddf6356..9309097 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -1,6 +1,6 @@
 .PHONY: all all_32 all_64 check_build32 clean run_tests
 
-TARGETS_C_BOTHBITS := sigreturn single_step_syscall
+TARGETS_C_BOTHBITS := sigreturn single_step_syscall sysret_ss_attrs
 
 BINARIES_32 := $(TARGETS_C_BOTHBITS:%=%_32)
 BINARIES_64 := $(TARGETS_C_BOTHBITS:%=%_64)
@@ -46,3 +46,6 @@ check_build32:
 	  echo "  yum install glibc-devel.*i686";			\
 	  exit 1;							\
 	fi
+
+# Some tests have additional dependencies.
+sysret_ss_attrs_64: thunks.S
diff --git a/tools/testing/selftests/x86/run_x86_tests.sh b/tools/testing/selftests/x86/run_x86_tests.sh
index 3fc19b3..d250342 100644
--- a/tools/testing/selftests/x86/run_x86_tests.sh
+++ b/tools/testing/selftests/x86/run_x86_tests.sh
@@ -4,10 +4,12 @@
 # script here.
 ./sigreturn_32 || exit 1
 ./single_step_syscall_32 || exit 1
+./sysret_ss_attrs_32 || exit 1
 
 if [[ "$uname -p" -eq "x86_64" ]]; then
     ./sigreturn_64 || exit 1
     ./single_step_syscall_64 || exit 1
+    ./sysret_ss_attrs_64 || exit 1
 fi
 
 exit 0
diff --git a/tools/testing/selftests/x86/sysret_ss_attrs.c b/tools/testing/selftests/x86/sysret_ss_attrs.c
new file mode 100644
index 0000000..ce42d5a
--- /dev/null
+++ b/tools/testing/selftests/x86/sysret_ss_attrs.c
@@ -0,0 +1,112 @@
+/*
+ * sysret_ss_attrs.c - test that syscalls return valid hidden SS attributes
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * On AMD CPUs, SYSRET can return with a valid SS descriptor with with
+ * the hidden attributes set to an unusable state.  Make sure the kernel
+ * doesn't let this happen.
+ */
+
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <err.h>
+#include <stddef.h>
+#include <stdbool.h>
+#include <pthread.h>
+
+static void *threadproc(void *ctx)
+{
+	/*
+	 * Do our best to cause sleeps on this CPU to exit the kernel and
+	 * re-enter with SS = 0.
+	 */
+	while (true)
+		;
+
+	return NULL;
+}
+
+#ifdef __x86_64__
+extern unsigned long call32_from_64(void *stack, void (*function)(void));
+
+asm (".pushsection .text\n\t"
+     ".code32\n\t"
+     "test_ss:\n\t"
+     "pushl $0\n\t"
+     "popl %eax\n\t"
+     "ret\n\t"
+     ".code64");
+extern void test_ss(void);
+#endif
+
+int main()
+{
+	/*
+	 * Start a busy-looping thread on the same CPU we're on.
+	 * For simplicity, just stick everything to CPU 0.  This will
+	 * fail in some containers, but that's probably okay.
+	 */
+	cpu_set_t cpuset;
+	CPU_ZERO(&cpuset);
+	CPU_SET(0, &cpuset);
+	if (sched_setaffinity(0, sizeof(cpuset), &cpuset) != 0)
+		printf("[WARN]\tsched_setaffinity failed\n");
+
+	pthread_t thread;
+	if (pthread_create(&thread, 0, threadproc, 0) != 0)
+		err(1, "pthread_create");
+
+#ifdef __x86_64__
+	unsigned char *stack32 = mmap(NULL, 4096, PROT_READ | PROT_WRITE,
+				      MAP_32BIT | MAP_ANONYMOUS | MAP_PRIVATE,
+				      -1, 0);
+	if (stack32 == MAP_FAILED)
+		err(1, "mmap");
+#endif
+
+	printf("[RUN]\tSyscalls followed by SS validation\n");
+
+	for (int i = 0; i < 1000; i++) {
+		/*
+		 * Go to sleep and return using sysret (if we're 64-bit
+		 * or we're 32-bit on AMD on a 64-bit kernel).  On AMD CPUs,
+		 * SYSRET doesn't fix up the cached SS descriptor, so the
+		 * kernel needs some kind of workaround to make sure that we
+		 * end the system call with a valid stack segment.  This
+		 * can be a confusing failure because the SS *selector*
+		 * is the same regardless.
+		 */
+		usleep(2);
+
+#ifdef __x86_64__
+		/*
+		 * On 32-bit, just doing a syscall through glibc is enough
+		 * to cause a crash if our cached SS descriptor is invalid.
+		 * On 64-bit, it's not, so try extra hard.
+		 */
+		call32_from_64(stack32 + 4088, test_ss);
+#endif
+	}
+
+	printf("[OK]\tWe survived\n");
+
+#ifdef __x86_64__
+	munmap(stack32, 4096);
+#endif
+
+	return 0;
+}
diff --git a/tools/testing/selftests/x86/thunks.S b/tools/testing/selftests/x86/thunks.S
new file mode 100644
index 0000000..ce8a995
--- /dev/null
+++ b/tools/testing/selftests/x86/thunks.S
@@ -0,0 +1,67 @@
+/*
+ * thunks.S - assembly helpers for mixed-bitness code
+ * Copyright (c) 2015 Andrew Lutomirski
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * These are little helpers that make it easier to switch bitness on
+ * the fly.
+ */
+
+	.text
+
+	.global call32_from_64
+	.type call32_from_64, @function
+call32_from_64:
+	// rdi: stack to use
+	// esi: function to call
+
+	// Save registers
+	pushq %rbx
+	pushq %rbp
+	pushq %r12
+	pushq %r13
+	pushq %r14
+	pushq %r15
+	pushfq
+
+	// Switch stacks
+	mov %rsp,(%rdi)
+	mov %rdi,%rsp
+
+	// Switch to compatibility mode
+	pushq $0x23  /* USER32_CS */
+	pushq $1f
+	lretq
+
+1:
+	.code32
+	// Call the function
+	call *%esi
+	// Switch back to long mode
+	jmp $0x33,$1f
+	.code64
+
+1:
+	// Restore the stack
+	mov (%rsp),%rsp
+
+	// Restore registers
+	popfq
+	popq %r15
+	popq %r14
+	popq %r13
+	popq %r12
+	popq %rbp
+	popq %rbx
+
+	ret
+
+.size call32_from_64, .-call32_from_64

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-05-08 13:31 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-24 22:09 [PATCH] x86, selftests: Add a test for the "sysret_ss_attrs" bug Andy Lutomirski
2015-04-25 13:55 ` Denys Vlasenko
2015-05-08 13:30 ` [tip:x86/asm] x86, selftests: Add a test for the "sysret_ss_attrs " bug tip-bot for Andy Lutomirski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).