[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/6] x86/AMD: Rework XSA-9 / Erratum 121 handling entirely
On Fri, Dec 28, 2018 at 12:39:33PM +0000, Andrew Cooper wrote: > 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"); Since you are switching the file to match Xen's coding style, I would use XENLOG_WARNING instead of KERN_WARNING. > + > /* > * 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; I think you can make this static now, since you have removed the only external user which was amd.c. Thanks, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |