From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754306AbbGPINh (ORCPT ); Thu, 16 Jul 2015 04:13:37 -0400 Received: from mga11.intel.com ([192.55.52.93]:21839 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751208AbbGPINd (ORCPT ); Thu, 16 Jul 2015 04:13:33 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.15,486,1432623600"; d="scan'208";a="763636157" Subject: [RFC][PATCH 1/2] x86, fpu: disable XSAVE if init buf too small To: dave@sr71.net Cc: torvalds@linux-foundation.org, mingo@kernel.org, linux-kernel@vger.kernel.org, luto@amacapital.net, bp@alien8.de, fenghua.yu@intel.com, hpa@zytor.com, oleg@redhat.com, tglx@linutronix.de, ross.zwisler@linux.intel.com From: Dave Hansen Date: Thu, 16 Jul 2015 01:13:32 -0700 Message-Id: <20150716081332.9413DD5B@viggo.jf.intel.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is _very_ lightly tested. Just RFC for now. The fallback code is not tested at all because I'm hitting another bug. -- The recent x86 FPU rework changed a dynamic allocation to a static one. But, the static one is undersized and we overrun it, corrupting memory in some cases. This patch detects the situation and disables the XSAVE feature. I have not been able to test this in practice because I'm hitting another bug. tsk->signal gets corrupted if I disable XSAVE. tsk->signal is conveniently located a few hundred bytes after the 'struct fpu' which is now embedded in task_struct, so I suspect this is another overrun. Anyone should be able to reproduce this by under-sizing XSTATE_MAX_SIZE in the next patch. Cc: Linus Torvalds Cc: Ingo Molnar Cc: Linux Kernel Mailing List Cc: Andy Lutomirski Cc: Borislav Petkov Cc: Fenghua Yu Cc: "H. Peter Anvin" Cc: leg Nesterov Cc: Thomas Gleixner Cc: Ross Zwisler --- b/arch/x86/include/asm/fpu/internal.h | 1 b/arch/x86/kernel/fpu/init.c | 17 ++++++++------ b/arch/x86/kernel/fpu/xstate.c | 41 +++++++++++++++++++++++----------- 3 files changed, 39 insertions(+), 20 deletions(-) diff -puN arch/x86/include/asm/fpu/internal.h~disable-xsave-if-init-buf-too-small arch/x86/include/asm/fpu/internal.h --- a/arch/x86/include/asm/fpu/internal.h~disable-xsave-if-init-buf-too-small 2015-07-16 01:11:02.889010467 -0700 +++ b/arch/x86/include/asm/fpu/internal.h 2015-07-16 01:11:33.928398118 -0700 @@ -42,6 +42,7 @@ extern void fpu__init_cpu_xstate(void); extern void fpu__init_system(struct cpuinfo_x86 *c); extern void fpu__init_check_bugs(void); extern void fpu__resume_cpu(void); +extern void fpu__clear_all_xsave_cpu_caps(void); /* * Debugging facility: diff -puN arch/x86/kernel/fpu/init.c~disable-xsave-if-init-buf-too-small arch/x86/kernel/fpu/init.c --- a/arch/x86/kernel/fpu/init.c~disable-xsave-if-init-buf-too-small 2015-07-16 01:11:02.890010512 -0700 +++ b/arch/x86/kernel/fpu/init.c 2015-07-16 01:11:02.896010780 -0700 @@ -301,20 +301,23 @@ static int __init no_387(char *s) } __setup("no387", no_387); -/* - * Disable all xstate CPU features: - */ -static int __init x86_noxsave_setup(char *s) +void fpu__clear_all_xsave_cpu_caps(void) { - if (strlen(s)) - return 0; - setup_clear_cpu_cap(X86_FEATURE_XSAVE); setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT); setup_clear_cpu_cap(X86_FEATURE_XSAVES); setup_clear_cpu_cap(X86_FEATURE_AVX); setup_clear_cpu_cap(X86_FEATURE_AVX2); +} +/* + * Disable all xstate CPU features: + */ +static int __init x86_noxsave_setup(char *s) +{ + if (strlen(s)) + return 0; + fpu__clear_all_xsave_cpu_caps(); return 1; } __setup("noxsave", x86_noxsave_setup); diff -puN arch/x86/kernel/fpu/xstate.c~disable-xsave-if-init-buf-too-small arch/x86/kernel/fpu/xstate.c --- a/arch/x86/kernel/fpu/xstate.c~disable-xsave-if-init-buf-too-small 2015-07-16 01:11:02.892010601 -0700 +++ b/arch/x86/kernel/fpu/xstate.c 2015-07-16 01:11:02.895010735 -0700 @@ -291,26 +291,37 @@ static void __init setup_init_fpu_buf(vo } /* - * Calculate total size of enabled xstates in XCR0/xfeatures_mask. + * Ask the CPU for the total size of the buffer needed to save all the + * enabled xstates in XCR0/xfeatures_mask. */ -static void __init init_xstate_size(void) +static int __init fetch_xstate_size(void) { unsigned int eax, ebx, ecx, edx; - int i; if (!cpu_has_xsaves) { + /* Standard format */ cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx); - xstate_size = ebx; - return; + } else { + /* Compacted format */ + cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx); } - xstate_size = FXSAVE_SIZE + XSAVE_HDR_SIZE; - for (i = 2; i < 64; i++) { - if (test_bit(i, (unsigned long *)&xfeatures_mask)) { - cpuid_count(XSTATE_CPUID, i, &eax, &ebx, &ecx, &edx); - xstate_size += eax; - } - } + return ebx; +} + +/* + * We did not allocate enough size in our init_fpstate to hold + * the xsave buffer on this system. Back out the xsave enabling + * we did so far and disable xsaves. + */ +static void xstate_size_overflow_error(void) +{ + pr_warn("x86/fpu: xstates too large (%d), disabling XSAVE\n", xstate_size); + /* Disable the xsave feature itself */ + cr4_clear_bits(X86_CR4_OSXSAVE); + + /* Clear the software copy of all the xsave-related cpuid bits */ + fpu__clear_all_xsave_cpu_caps(); } /* @@ -350,7 +361,11 @@ void __init fpu__init_system_xstate(void fpu__init_cpu_xstate(); /* Recompute the context size for enabled features: */ - init_xstate_size(); + xstate_size = fetch_xstate_size(); + if (xstate_size > sizeof(init_fpstate.xsave)) { + xstate_size_overflow_error(); + return; + } update_regset_xstate_info(xstate_size, xfeatures_mask); fpu__init_prepare_fx_sw_frame(); _