| 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
| ||
|
Message-ID: <CAP045ArmdQhULh36k3b72-QNQ+BfNbw6We+iLM_WdVKRBf9s4Q@mail.gmail.com>
Date: Wed, 14 Sep 2016 18:47:25 -0700
From: Kyle Huey <me@...ehuey.com>
To: Andy Lutomirski <luto@...capital.net>
Cc: "Robert O'Callahan" <robert@...llahan.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>,
Shuah Khan <shuah@...nel.org>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>,
Borislav Petkov <bp@...e.de>,
"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
Len Brown <len.brown@...el.com>,
Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
Aravind Gopalakrishnan <Aravind.Gopalakrishnan@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Vladimir Zapolskiy <vladimir_zapolskiy@...tor.com>,
Andy Lutomirski <luto@...nel.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Dmitry Safonov <dsafonov@...tuozzo.com>,
Kees Cook <keescook@...omium.org>,
"Michael S. Tsirkin" <mst@...hat.com>,
Andrey Ryabinin <aryabinin@...tuozzo.com>,
Jiri Slaby <jslaby@...e.cz>,
Paul Gortmaker <paul.gortmaker@...driver.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
"open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<linux-kernel@...r.kernel.org>,
"open list:KERNEL SELFTEST FRAMEWORK"
<linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] x86,arch_prctl Add ARCH_[GET|SET]_CPUID for
controlling the CPUID instruction
On Wed, Sep 14, 2016 at 6:29 PM, Andy Lutomirski <luto@...capital.net> wrote:
> On Wed, Sep 14, 2016 at 2:01 PM, Kyle Huey <me@...ehuey.com> 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 <khuey@...ehuey.com>
>> ---
>> 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
Powered by blists - more mailing lists