From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933558AbcIOBr3 (ORCPT ); Wed, 14 Sep 2016 21:47:29 -0400 Received: from mail-vk0-f65.google.com ([209.85.213.65]:36225 "EHLO mail-vk0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756781AbcIOBr1 (ORCPT ); Wed, 14 Sep 2016 21:47:27 -0400 MIME-Version: 1.0 In-Reply-To: References: <1473886902-17902-1-git-send-email-khuey@kylehuey.com> <1473886902-17902-4-git-send-email-khuey@kylehuey.com> From: Kyle Huey Date: Wed, 14 Sep 2016 18:47:25 -0700 Message-ID: Subject: Re: [PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for controlling the CPUID instruction To: Andy Lutomirski Cc: "Robert O'Callahan" , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" , Shuah Khan , "Peter Zijlstra (Intel)" , Borislav Petkov , "Rafael J. Wysocki" , Len Brown , Srinivas Pandruvada , Aravind Gopalakrishnan , Alexander Shishkin , Vladimir Zapolskiy , Andy Lutomirski , Frederic Weisbecker , Dmitry Safonov , Kees Cook , "Michael S. Tsirkin" , Andrey Ryabinin , Jiri Slaby , Paul Gortmaker , Denys Vlasenko , Dave Hansen , "open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)" , "open list:KERNEL SELFTEST FRAMEWORK" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 14, 2016 at 6:29 PM, Andy Lutomirski wrote: > On Wed, Sep 14, 2016 at 2:01 PM, Kyle Huey wrote: >> Intel supports faulting on the CPUID instruction in newer processors. Bit >> 31 of MSR_PLATFORM_INFO advertises support for this feature. It is >> documented in detail in Section 2.3.2 of >> http://www.intel.com/content/dam/www/public/us/en/documents/application-notes/virtualization-technology-flexmigration-application-note.pdf >> >> Signed-off-by: Kyle Huey >> --- >> arch/x86/include/asm/msr-index.h | 1 + >> arch/x86/include/asm/thread_info.h | 4 +- >> arch/x86/include/uapi/asm/prctl.h | 6 + >> arch/x86/kernel/process.c | 81 +++++++++++ >> tools/testing/selftests/x86/Makefile | 2 +- >> tools/testing/selftests/x86/cpuid-fault.c | 223 ++++++++++++++++++++++++++++++ >> 6 files changed, 315 insertions(+), 2 deletions(-) >> create mode 100644 tools/testing/selftests/x86/cpuid-fault.c >> >> diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h >> index 83908d5..4aebec2 100644 >> --- a/arch/x86/include/asm/msr-index.h >> +++ b/arch/x86/include/asm/msr-index.h >> @@ -53,6 +53,7 @@ >> #define MSR_MTRRcap 0x000000fe >> #define MSR_IA32_BBL_CR_CTL 0x00000119 >> #define MSR_IA32_BBL_CR_CTL3 0x0000011e >> +#define MSR_MISC_FEATURES_ENABLES 0x00000140 >> >> #define MSR_IA32_SYSENTER_CS 0x00000174 >> #define MSR_IA32_SYSENTER_ESP 0x00000175 >> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h >> index 8b7c8d8..ec93976 100644 >> --- a/arch/x86/include/asm/thread_info.h >> +++ b/arch/x86/include/asm/thread_info.h >> @@ -93,6 +93,7 @@ struct thread_info { >> #define TIF_SECCOMP 8 /* secure computing */ >> #define TIF_USER_RETURN_NOTIFY 11 /* notify kernel of userspace return */ >> #define TIF_UPROBE 12 /* breakpointed or singlestepping */ >> +#define TIF_NOCPUID 15 /* CPUID is not accessible in userland */ >> #define TIF_NOTSC 16 /* TSC is not accessible in userland */ >> #define TIF_IA32 17 /* IA32 compatibility process */ >> #define TIF_FORK 18 /* ret_from_fork */ >> @@ -117,6 +118,7 @@ struct thread_info { >> #define _TIF_SECCOMP (1 << TIF_SECCOMP) >> #define _TIF_USER_RETURN_NOTIFY (1 << TIF_USER_RETURN_NOTIFY) >> #define _TIF_UPROBE (1 << TIF_UPROBE) >> +#define _TIF_NOCPUID (1 << TIF_NOCPUID) >> #define _TIF_NOTSC (1 << TIF_NOTSC) >> #define _TIF_IA32 (1 << TIF_IA32) >> #define _TIF_FORK (1 << TIF_FORK) >> @@ -146,7 +148,7 @@ struct thread_info { >> >> /* flags to check in __switch_to() */ >> #define _TIF_WORK_CTXSW \ >> - (_TIF_IO_BITMAP|_TIF_NOTSC|_TIF_BLOCKSTEP) >> + (_TIF_IO_BITMAP|_TIF_NOCPUID|_TIF_NOTSC|_TIF_BLOCKSTEP) >> >> #define _TIF_WORK_CTXSW_PREV (_TIF_WORK_CTXSW|_TIF_USER_RETURN_NOTIFY) >> #define _TIF_WORK_CTXSW_NEXT (_TIF_WORK_CTXSW) >> diff --git a/arch/x86/include/uapi/asm/prctl.h b/arch/x86/include/uapi/asm/prctl.h >> index 3ac5032..c087e55 100644 >> --- a/arch/x86/include/uapi/asm/prctl.h >> +++ b/arch/x86/include/uapi/asm/prctl.h >> @@ -6,4 +6,10 @@ >> #define ARCH_GET_FS 0x1003 >> #define ARCH_GET_GS 0x1004 >> >> +/* Get/set the process' ability to use the CPUID instruction */ >> +#define ARCH_GET_CPUID 0x1005 >> +#define ARCH_SET_CPUID 0x1006 >> +# define ARCH_CPUID_ENABLE 1 /* allow the use of the CPUID instruction */ >> +# define ARCH_CPUID_SIGSEGV 2 /* throw a SIGSEGV instead of reading the CPUID */ >> + >> #endif /* _ASM_X86_PRCTL_H */ >> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c >> index 0f857c3..5fc8e9d 100644 >> --- a/arch/x86/kernel/process.c >> +++ b/arch/x86/kernel/process.c >> @@ -193,6 +193,69 @@ int set_tsc_mode(unsigned int val) >> return 0; >> } >> >> +static void switch_cpuid_faulting(bool on) >> +{ >> + if (on) >> + msr_set_bit(MSR_MISC_FEATURES_ENABLES, 0); >> + else >> + msr_clear_bit(MSR_MISC_FEATURES_ENABLES, 0); >> +} >> + > > I don't know if we care (yet?), but this is going to be unnecessarily > slow because of the implicit rdmsr. You could add a percpu shadow > copy of MISC_FEATURES_ENABLES, initialized during boot, and avoid the > rdmsr. > >> + >> +int set_cpuid_mode(struct task_struct *task, unsigned long val) >> +{ >> + /* Only disable/enable_cpuid() if it is supported on this hardware. */ >> + bool cpuid_fault_supported = static_cpu_has(X86_FEATURE_CPUID_FAULT); >> + >> + if (val == ARCH_CPUID_ENABLE && cpuid_fault_supported) { >> + if (task_no_new_privs(task) && test_thread_flag(TIF_NOCPUID)) >> + return -EACCES; > > This check seems confused. If this flag were preserved on execve, > it's the SIGSEGV mode that would need the check. Not sure I follow this one. no_new_privs should block transitions from SIGSEGV to ENABLE, right? That's what this check does. >> @@ -212,6 +275,15 @@ void __switch_to_xtra(struct task_struct *prev_p, struct task_struct *next_p, >> update_debugctlmsr(debugctl); >> } >> >> + if (test_tsk_thread_flag(prev_p, TIF_NOCPUID) ^ >> + test_tsk_thread_flag(next_p, TIF_NOCPUID)) { >> + /* prev and next are different */ >> + if (test_tsk_thread_flag(next_p, TIF_NOCPUID)) >> + switch_cpuid_faulting(true); >> + else >> + switch_cpuid_faulting(false); >> + } >> + > > Off-topic and not needed for this patch, but IMO we should move all of > this context switch junk out of ti.flags and into thread_struct > somewhere. - Kyle