| 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
| ||
|
Date: Wed, 14 Sep 2016 18:29:26 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Kyle Huey <me@...ehuey.com>
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 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.
> @@ -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.
Powered by blists - more mailing lists