[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-devel] [PATCH 3/6] x86/AMD: Rework XSA-9 / Erratum 121 handling entirely



There are multiple problems:

 * The opt_allow_unsafe < 0 logic is dead since 2012 (c/s 0c7a6966511
   "x86-64: refine the XSA-9 fix").
 * Given that opt_allow_unsafe was deliberately intended not to be
   specific to #121 alone, setting it to true for the not-affected case
   will cause a security issue if a second use of this option ever
   appears.
 * Calling cpu_has_amd_erratum() on every domain creation is wasteful,
   given that the answer is static after boot.

Move opt_allow_unsafe into domain.c, as a better location for it to
live, and switch it to be a straight boolean.

Use the new cpu_bug_* infrastructure to precompute erratum 121 during
boot, rather than repeatedly at runtime.  Leave a comment beside the
check in arch_domain_create() to explain why we may refuse to boot
DomU's.

Reflow the printed information for grep-ability, and fix them for
correctness and brevity.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wei.liu2@xxxxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/cpu/amd.c            | 26 ++++++++------------------
 xen/arch/x86/domain.c             | 19 +++++++++++++------
 xen/include/asm-x86/amd.h         |  5 -----
 xen/include/asm-x86/cpufeature.h  |  3 +++
 xen/include/asm-x86/cpufeatures.h |  2 ++
 xen/include/asm-x86/domain.h      |  2 ++
 6 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/xen/arch/x86/cpu/amd.c b/xen/arch/x86/cpu/amd.c
index c3aa1f4..8089fb9 100644
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -40,10 +40,6 @@ integer_param("cpuid_mask_l7s0_ebx", 
opt_cpuid_mask_l7s0_ebx);
 static unsigned int __initdata opt_cpuid_mask_thermal_ecx = ~0u;
 integer_param("cpuid_mask_thermal_ecx", opt_cpuid_mask_thermal_ecx);
 
-/* 1 = allow, 0 = don't allow guest creation, -1 = don't allow boot */
-s8 __read_mostly opt_allow_unsafe;
-boolean_param("allow_unsafe", opt_allow_unsafe);
-
 /* Signal whether the ACPI C1E quirk is required. */
 bool __read_mostly amd_acpi_c1e_quirk;
 
@@ -538,6 +534,14 @@ static void init_amd_k8(struct cpuinfo_x86 *c)
 {
     uint64_t val;
 
+    setup_force_cpu_cap(X86_BUG_AMD_ERRATUM_121);
+
+    if ( c == &boot_cpu_data && !opt_allow_unsafe )
+        printk(KERN_WARNING
+               "*** Xen will not allow DomU creation on this CPU for security 
reasons ***\n"
+               KERN_WARNING
+               "*** Pass \"allow_unsafe\" if you trust all your guest kernels 
***\n");
+
     /*
      * Skip errata workarounds if we are virtualised.  We won't have
      * sufficient control of hardware to do anything useful.
@@ -784,20 +788,6 @@ static void init_amd(struct cpuinfo_x86 *c)
 
         amd_get_topology(c);
 
-       if (!cpu_has_amd_erratum(c, AMD_ERRATUM_121))
-               opt_allow_unsafe = 1;
-       else if (opt_allow_unsafe < 0)
-               panic("Xen will not boot on this CPU for security reasons"
-                     "Pass \"allow_unsafe\" if you're trusting all your"
-                     " (PV) guest kernels.\n");
-       else if (!opt_allow_unsafe && c == &boot_cpu_data)
-               printk(KERN_WARNING
-                      "*** Xen will not allow creation of DomU-s on"
-                      " this CPU for security reasons. ***\n"
-                      KERN_WARNING
-                      "*** Pass \"allow_unsafe\" if you're trusting"
-                      " all your (PV) guest kernels. ***\n");
-
        /* AMD CPUs do not support SYSENTER outside of legacy mode. */
        __clear_bit(X86_FEATURE_SEP, c->x86_capability);
 
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 32dc4253..beeb1d7 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -71,6 +71,10 @@
 
 DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
 
+/* Permit creating domains on unsafe systems? */
+bool __read_mostly opt_allow_unsafe;
+boolean_param("allow_unsafe", opt_allow_unsafe);
+
 static void default_idle(void);
 void (*pm_idle) (void) __read_mostly = default_idle;
 void (*dead_idle) (void) __read_mostly = default_dead_idle;
@@ -506,17 +510,20 @@ int arch_domain_create(struct domain *d,
         return -EINVAL;
     }
 
-    if ( d->domain_id && cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) )
+    /*
+     * AMD Erratum 121 results in a core hang when executing into the
+     * non-canonical region.  64bit PV and HVM guests can pull this off, and
+     * there is nothing Xen can do to mitigate.
+     */
+    if ( unlikely(cpu_bug_amd_erratum_121) && d->domain_id )
     {
         if ( !opt_allow_unsafe )
         {
-            printk(XENLOG_G_ERR "Xen does not allow DomU creation on this CPU"
-                   " for security reasons.\n");
+            printk(XENLOG_G_ERR
+                   "Xen does not allow DomU creation on this CPU for security 
reasons\n");
             return -EPERM;
         }
-        printk(XENLOG_G_WARNING
-               "Dom%d may compromise security on this CPU.\n",
-               d->domain_id);
+        printk(XENLOG_G_WARNING "%pd may compromise security on this CPU\n", 
d);
     }
 
     d->arch.s3_integrity = config->flags & XEN_DOMCTL_CDF_s3_integrity;
diff --git a/xen/include/asm-x86/amd.h b/xen/include/asm-x86/amd.h
index a82382e..3d82db1 100644
--- a/xen/include/asm-x86/amd.h
+++ b/xen/include/asm-x86/amd.h
@@ -124,9 +124,6 @@
 #define AMD_MODEL_RANGE_START(range)    (((range) >> 12) & 0xfff)
 #define AMD_MODEL_RANGE_END(range)      ((range) & 0xfff)
 
-#define AMD_ERRATUM_121                                                 \
-    AMD_LEGACY_ERRATUM(AMD_MODEL_RANGE(0x0f, 0x0, 0x0, 0x3f, 0xf))
-
 #define AMD_ERRATUM_170                                                 \
     AMD_LEGACY_ERRATUM(AMD_MODEL_RANGE(0x0f, 0x0, 0x0, 0x67, 0xf))
 
@@ -143,8 +140,6 @@
 struct cpuinfo_x86;
 int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
 
-extern s8 opt_allow_unsafe;
-
 void fam10h_check_enable_mmcfg(void);
 void check_enable_amd_mmconf_dmi(void);
 
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 5592e17..4ed7be3 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -122,6 +122,9 @@
 
 #define cpu_has_msr_tsc_aux     (cpu_has_rdtscp || cpu_has_rdpid)
 
+/* Bugs. */
+#define cpu_bug_amd_erratum_121 boot_cpu_has(X86_BUG_AMD_ERRATUM_121)
+
 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 ba468ea..a19116c 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -38,5 +38,7 @@ XEN_CPUFEATURE(XEN_LBR,           X86_SYNTH(22)) /* Xen uses 
MSR_DEBUGCTL.LBR */
 #define X86_NR_BUG 1
 #define X86_BUG(x) ((FSCAPINTS + X86_NR_SYNTH) * 32 + (x))
 
+#define X86_BUG_AMD_ERRATUM_121   X86_BUG( 0) /* Hang on fetch across 
non-canonical boundary. */
+
 /* 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/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index 277f99f..62bafe3 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -12,6 +12,8 @@
 #include <public/vcpu.h>
 #include <public/hvm/hvm_info_table.h>
 
+extern bool opt_allow_unsafe;
+
 #define has_32bit_shinfo(d)    ((d)->arch.has_32bit_shinfo)
 
 #define is_hvm_pv_evtchn_domain(d) (is_hvm_domain(d) && \
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.