From patchwork Mon Aug 19 18:26:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew Cooper X-Patchwork-Id: 11101649 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 671A8912 for ; Mon, 19 Aug 2019 18:28:01 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3AD1A218BA for ; Mon, 19 Aug 2019 18:28:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=citrix.com header.i=@citrix.com header.b="E1AFXPRC" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3AD1A218BA Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=citrix.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=xen-devel-bounces@lists.xenproject.org Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hzmMc-0003wM-0O; Mon, 19 Aug 2019 18:26:42 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1hzmMb-0003w3-1p for xen-devel@lists.xenproject.org; Mon, 19 Aug 2019 18:26:41 +0000 X-Inumbo-ID: e2aa2dac-c2ae-11e9-8bf0-12813bfff9fa Received: from esa1.hc3370-68.iphmx.com (unknown [216.71.145.142]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTPS id e2aa2dac-c2ae-11e9-8bf0-12813bfff9fa; Mon, 19 Aug 2019 18:26:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=citrix.com; s=securemail; t=1566239200; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=lKw4kw/gjVqAVUR64oVZzTy4/hEHxD592hRXVtS05Qo=; b=E1AFXPRCIiDswQ5mpGMZLvquBg6ARhlPM+vjTUBihio4ikiHmMSQ3519 VJ4aFL65GxwCYb7nzBT8mfM7WkbNT3V72PuyZL06EgpsXotEAPuFqAstp t955RFL2vcDFNSJGJxc9xwBuXXSqNPrFcU4X5/8yJiYHhmhMylfO2wx/m g=; Authentication-Results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=andrew.cooper3@citrix.com; spf=Pass smtp.mailfrom=Andrew.Cooper3@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of andrew.cooper3@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="andrew.cooper3@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa1.hc3370-68.iphmx.com: domain of Andrew.Cooper3@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="Andrew.Cooper3@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ~all" Received-SPF: None (esa1.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa1.hc3370-68.iphmx.com; envelope-from="Andrew.Cooper3@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: Ag6ydAjPr6TkZPbJnqXPQ1ln97qAxKttPfAEF3froGw6lvH29h3XRX6CiBzwZQiQaYWI7QidpV fzSQdRGpvCglgPlYmMCr7ulzOYJ4zmpTLs5ZRCM1GcUoJhOB2WXv3BBEYGYp1ebiNxbunZG4Aw boBo3TQwc1HpQztRtS1WIrZH5ZhU3a9bDCukNwECQUqlIRhKqVIuclEtailFRJYqAPaUTxoRXb 10db1Khi5toYXC8BBP17/3BxUTzJS/VRE70C1Kyme1UTUkyD71b21n6l1I7WHSS5XpOwyYbebS HO0= X-SBRS: 2.7 X-MesageID: 4484232 X-Ironport-Server: esa1.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.64,405,1559534400"; d="scan'208";a="4484232" From: Andrew Cooper To: Xen-devel Date: Mon, 19 Aug 2019 19:26:12 +0100 Message-ID: <20190819182612.16706-3-andrew.cooper3@citrix.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20190819182612.16706-1-andrew.cooper3@citrix.com> References: <20190819182612.16706-1-andrew.cooper3@citrix.com> MIME-Version: 1.0 Subject: [Xen-devel] [PATCH 2/2] x86/AMD: Fix handling of x87 exception pointers on Fam17h hardware X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Andrew Cooper , Wei Liu , Jan Beulich , =?utf-8?q?Roger_Pau_Monn=C3=A9?= Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" AMD Pre-Fam17h CPUs "optimise" {F,}X{SAVE,RSTOR} by not saving/restoring FOP/FIP/FDP if an x87 exception isn't pending. This causes an information leak, CVE-2006-1056, and worked around by several OSes, including Xen. AMD Fam17h CPUs no longer have this leak, and advertise so in a CPUID bit. Introduce the RSTR_FP_ERR_PTRS feature, as specified by AMD, and expose to all guests by default. While adjusting libxl's cpuid table, add CLZERO which looks to have been omitted previously. Also introduce an X86_BUG bit to trigger the (F)XRSTOR workaround, and set it on AMD hardware where RSTR_FP_ERR_PTRS is not advertised. Optimise the workaround path by dropping the data-dependent unpredictable conditions which will evalute to true for all 64bit OSes and most 32bit ones. Signed-off-by: Andrew Cooper --- CC: Jan Beulich CC: Wei Liu CC: Roger Pau Monné v2: * Use the AMD naming, not that I am convinced this is a sensible name to use. * Adjust the i387 codepaths as well as the xstate ones. * Add xen-cpuid/libxl data for the CPUID bit. --- tools/libxl/libxl_cpuid.c | 3 +++ tools/misc/xen-cpuid.c | 1 + xen/arch/x86/cpu/amd.c | 7 +++++++ xen/arch/x86/i387.c | 14 +++++--------- xen/arch/x86/xstate.c | 6 ++---- xen/include/asm-x86/cpufeature.h | 3 +++ xen/include/asm-x86/cpufeatures.h | 2 ++ xen/include/public/arch-x86/cpufeatureset.h | 1 + 8 files changed, 24 insertions(+), 13 deletions(-) diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c index a8d07fac50..acc92fd46c 100644 --- a/tools/libxl/libxl_cpuid.c +++ b/tools/libxl/libxl_cpuid.c @@ -256,7 +256,10 @@ int libxl_cpuid_parse_config(libxl_cpuid_policy_list *cpuid, const char* str) {"invtsc", 0x80000007, NA, CPUID_REG_EDX, 8, 1}, + {"clzero", 0x80000008, NA, CPUID_REG_EBX, 0, 1}, + {"rstr-fp-err-ptrs", 0x80000008, NA, CPUID_REG_EBX, 2, 1}, {"ibpb", 0x80000008, NA, CPUID_REG_EBX, 12, 1}, + {"nc", 0x80000008, NA, CPUID_REG_ECX, 0, 8}, {"apicidsize", 0x80000008, NA, CPUID_REG_ECX, 12, 4}, diff --git a/tools/misc/xen-cpuid.c b/tools/misc/xen-cpuid.c index b0db0525a9..04cdd9aa95 100644 --- a/tools/misc/xen-cpuid.c +++ b/tools/misc/xen-cpuid.c @@ -145,6 +145,7 @@ static const char *const str_e7d[32] = static const char *const str_e8b[32] = { [ 0] = "clzero", + [ 2] = "rstr-fp-err-ptrs", [12] = "ibpb", }; diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c index a2f83c79a5..463f9776c7 100644 --- a/xen/arch/x86/cpu/amd.c +++ b/xen/arch/x86/cpu/amd.c @@ -580,6 +580,13 @@ static void init_amd(struct cpuinfo_x86 *c) } /* + * Older AMD CPUs don't save/load FOP/FIP/FDP unless an FPU exception + * is pending. Xen works around this at (F)XRSTOR time. + */ + if ( !cpu_has(c, X86_FEATURE_RSTR_FP_ERR_PTRS) ) + setup_force_cpu_cap(X86_BUG_FPU_PTR_LEAK); + + /* * Attempt to set lfence to be Dispatch Serialising. This MSR almost * certainly isn't virtualised (and Xen at least will leak the real * value in but silently discard writes), as well as being per-core diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c index 88178485cb..82dbc461c3 100644 --- a/xen/arch/x86/i387.c +++ b/xen/arch/x86/i387.c @@ -43,20 +43,17 @@ static inline void fpu_fxrstor(struct vcpu *v) const typeof(v->arch.xsave_area->fpu_sse) *fpu_ctxt = v->arch.fpu_ctxt; /* - * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception + * Some CPUs don't save/restore FDP/FIP/FOP unless an exception * is pending. Clear the x87 state here by setting it to fixed * values. The hypervisor data segment can be sometimes 0 and * sometimes new user value. Both should be ok. Use the FPU saved * data block as a safe address because it should be in L1. */ - if ( !(fpu_ctxt->fsw & ~fpu_ctxt->fcw & 0x003f) && - boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) - { + if ( cpu_bug_fpu_ptr_leak ) asm volatile ( "fnclex\n\t" "ffree %%st(7)\n\t" /* clear stack tag */ "fildl %0" /* load to clear state */ : : "m" (*fpu_ctxt) ); - } /* * FXRSTOR can fault if passed a corrupted data block. We handle this @@ -169,11 +166,10 @@ static inline void fpu_fxsave(struct vcpu *v) : "=m" (*fpu_ctxt) : "R" (fpu_ctxt) ); /* - * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception - * is pending. + * Some CPUs don't save/restore FDP/FIP/FOP unless an exception is + * pending. The restore code fills in suitable defaults. */ - if ( !(fpu_ctxt->fsw & 0x0080) && - boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) + if ( cpu_bug_fpu_ptr_leak && !(fpu_ctxt->fsw & 0x0080) ) return; /* diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c index 3293ef834f..fd3c0c5a36 100644 --- a/xen/arch/x86/xstate.c +++ b/xen/arch/x86/xstate.c @@ -369,15 +369,13 @@ void xrstor(struct vcpu *v, uint64_t mask) unsigned int faults, prev_faults; /* - * AMD CPUs don't save/restore FDP/FIP/FOP unless an exception + * Some CPUs don't save/restore FDP/FIP/FOP unless an exception * is pending. Clear the x87 state here by setting it to fixed * values. The hypervisor data segment can be sometimes 0 and * sometimes new user value. Both should be ok. Use the FPU saved * data block as a safe address because it should be in L1. */ - if ( (mask & ptr->xsave_hdr.xstate_bv & X86_XCR0_FP) && - !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw & 0x003f) && - boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) + if ( cpu_bug_fpu_ptr_leak ) asm volatile ( "fnclex\n\t" /* clear exceptions */ "ffree %%st(7)\n\t" /* clear stack tag */ "fildl %0" /* load to clear state */ diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h index 906dd59c4b..5d7b819314 100644 --- a/xen/include/asm-x86/cpufeature.h +++ b/xen/include/asm-x86/cpufeature.h @@ -136,6 +136,9 @@ #define cpu_has_msr_tsc_aux (cpu_has_rdtscp || cpu_has_rdpid) +/* Bugs. */ +#define cpu_bug_fpu_ptr_leak boot_cpu_has(X86_BUG_FPU_PTR_LEAK) + enum _cache_type { CACHE_TYPE_NULL = 0, CACHE_TYPE_DATA = 1, diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h index ab3650f73b..afb861f588 100644 --- a/xen/include/asm-x86/cpufeatures.h +++ b/xen/include/asm-x86/cpufeatures.h @@ -43,5 +43,7 @@ XEN_CPUFEATURE(SC_VERW_IDLE, X86_SYNTH(25)) /* VERW used by Xen for idle */ #define X86_NR_BUG 1 #define X86_BUG(x) ((FSCAPINTS + X86_NR_SYNTH) * 32 + (x)) +#define X86_BUG_FPU_PTR_LEAK X86_BUG( 0) /* (F)XRSTOR doesn't load FOP/FIP/FDP. */ + /* Total number of capability words, inc synth and bug words. */ #define NCAPINTS (FSCAPINTS + X86_NR_SYNTH + X86_NR_BUG) /* N 32-bit words worth of info */ diff --git a/xen/include/public/arch-x86/cpufeatureset.h b/xen/include/public/arch-x86/cpufeatureset.h index e2c82a4554..babaf4b375 100644 --- a/xen/include/public/arch-x86/cpufeatureset.h +++ b/xen/include/public/arch-x86/cpufeatureset.h @@ -243,6 +243,7 @@ XEN_CPUFEATURE(EFRO, 7*32+10) /* APERF/MPERF Read Only interface */ /* AMD-defined CPU features, CPUID level 0x80000008.ebx, word 8 */ XEN_CPUFEATURE(CLZERO, 8*32+ 0) /*A CLZERO instruction */ +XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers. */ XEN_CPUFEATURE(IBPB, 8*32+12) /*A IBPB support only (no IBRS, used by AMD) */ /* Intel-defined CPU features, CPUID level 0x00000007:0.edx, word 9 */