From e22438f8e997ac1c8911d8808b6a4c492cd8bc6e Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Fri, 24 Apr 2015 15:09:19 -0700 Subject: 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 Tested-by: Denys Vlasenko Signed-off-by: Andy Lutomirski Tests: e7d6eefaaa44 ("x86/vdso32/syscall.S: Do not load __USER32_DS to %ss") Cc: Alexei Starovoitov Cc: Andrew Morton Cc: Borislav Petkov Cc: Denys Vlasenko Cc: Frederic Weisbecker Cc: Kees Cook Cc: Linus Torvalds Cc: Oleg Nesterov Cc: Peter Zijlstra Cc: Steven Rostedt Cc: Thomas Gleixner Cc: Will Drewry Link: http://lkml.kernel.org/r/resend_4d740841bac383742949e2fefb03982736595087.git.luto@kernel.org Signed-off-by: Ingo Molnar --- 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(-) create mode 100644 tools/testing/selftests/x86/sysret_ss_attrs.c create mode 100644 tools/testing/selftests/x86/thunks.S (limited to 'tools') 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 +#include +#include +#include +#include +#include +#include +#include +#include + +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 -- cgit v1.1 From b72e7464e4cf80117938e6adb8c22fdc1ca46d42 Mon Sep 17 00:00:00 2001 From: Borislav Petkov Date: Thu, 4 Jun 2015 18:55:26 +0200 Subject: x86/uapi: Do not export as part of the user API headers This header containing all MSRs and respective bit definitions got exported to userspace in conjunction with the big UAPI shuffle. But, it doesn't belong in the UAPI headers because userspace can do its own MSR defines and exporting them from the kernel blocks us from doing cleanups/renames in that header. Which is ridiculous - it is not kernel's job to export such a header and keep MSRs list and their names stable. Signed-off-by: Borislav Petkov Acked-by: H. Peter Anvin Cc: Andrew Morton Cc: Andy Lutomirski Cc: David Howells Cc: Linus Torvalds Cc: Peter Zijlstra (Intel) Cc: Peter Zijlstra Cc: Thomas Gleixner Link: http://lkml.kernel.org/r/1433436928-31903-19-git-send-email-bp@alien8.de Signed-off-by: Ingo Molnar --- tools/power/x86/turbostat/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/power/x86/turbostat/Makefile b/tools/power/x86/turbostat/Makefile index 4039854..e367b1a8 100644 --- a/tools/power/x86/turbostat/Makefile +++ b/tools/power/x86/turbostat/Makefile @@ -9,7 +9,7 @@ endif turbostat : turbostat.c CFLAGS += -Wall -CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/uapi/asm/msr-index.h"' +CFLAGS += -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' %: %.c @mkdir -p $(BUILD_OUTPUT) -- cgit v1.1