x86/AMD: Fix handling of x87 exception pointers on Fam17h hardware

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
conditions for the workaround paths.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

diff --git a/tools/libxl/libxl_cpuid.c b/tools/libxl/libxl_cpuid.c
index f1c6ce2076e150662c3135cfd65671001082a797..953a3bbd8ccbd5dd133cca8d98952bf7f0dbce01 100644 (file)
--- a/tools/libxl/libxl_cpuid.c
+++ b/tools/libxl/libxl_cpuid.c
@@ -257,8 +257,11 @@ 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},
         {"wbnoinvd",     0x80000008, NA, CPUID_REG_EBX,  9,  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 be6a8d27a5d2c3640910be4d3ba960d1e9ce38aa..f51facffb6ec71eb1dd36751f9c687ab10f5f408 100644 (file)
--- 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",
 
     /* [ 8] */            [ 9] = "wbnoinvd",
 

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index a2f83c79a54042e9a3c076accc4d04fbf8c14922..fec2830c6ae5e49674ce14b160e6bbaa80fdee13 100644 (file)
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -579,6 +579,13 @@ static void init_amd(struct cpuinfo_x86 *c)
                        wrmsr_amd_safe(0xc001100d, l, h & ~1);
        }
 
+       /*
+        * 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_PTRS);
+
        /*
         * Attempt to set lfence to be Dispatch Serialising.  This MSR almost
         * certainly isn't virtualised (and Xen at least will leak the real

diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c
index 88178485cb8261f73fd44630420460e6d7f8d2b5..677f571792fe4f6f83cf9ee758fa4899377a16a9 100644 (file)
--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -43,20 +43,18 @@ 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_ptrs &&
+         !(fpu_ctxt->fsw & ~fpu_ctxt->fcw & 0x003f) )
         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 +167,11 @@ 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.  In this case, the restore side will arrange safe values,
+         * and there is no point trying to collect FCS/FDS in addition.
          */
-        if ( !(fpu_ctxt->fsw & 0x0080) &&
-             boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+        if ( cpu_bug_fpu_ptrs && !(fpu_ctxt->fsw & 0x0080) )
             return;
 
         /*

diff --git a/xen/arch/x86/xstate.c b/xen/arch/x86/xstate.c
index 3293ef834f9b41f824faca69ced8aa7e0efb234c..10016a05d0d1bd88f783ba184e5857f20a3b10d8 100644 (file)
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -369,15 +369,14 @@ 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_ptrs &&
+         !(ptr->fpu_sse.fsw & ~ptr->fpu_sse.fcw & 0x003f) )
         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 7e1ff17ad46c79925e8842da7936b6c8052c6146..00d22caac74c69b2462935986b52480f91b91ec5 100644 (file)
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -138,6 +138,9 @@
 
 #define cpu_has_msr_tsc_aux     (cpu_has_rdtscp || cpu_has_rdpid)
 
+/* Bugs. */
+#define cpu_bug_fpu_ptrs        boot_cpu_has(X86_BUG_FPU_PTRS)
+
 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 ab3650f73b9e8abe56056217284664126b399ade..91eccf5161f733a4ff2eefc9fa97d07c253d79e8 100644 (file)
--- 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_PTRS          X86_BUG( 0) /* (F)X{SAVE,RSTOR} doesn't save/restore 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 f2ec4701792244d5516445e183d36009ae6e1642..48d8d1f4e2fd4565e19183b9e3fd0e1c06503aab 100644 (file)
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -244,6 +244,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(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
 XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */