Our product management wasn''t happy with the "solution" for
XSA-9, and
demanded that customer systems must continue to boot. Rather than
having our and perhaps other distros carry non-trivial patches, allow
for more fine grained control (panic on boot, deny guest creation, or
merely warn) by means of a single line change.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/cpu/amd.c
+++ b/xen/arch/x86/cpu/amd.c
@@ -32,8 +32,11 @@
static char opt_famrev[14];
string_param("cpuid_mask_cpu", opt_famrev);
-static bool_t opt_allow_unsafe;
+#ifdef __x86_64__
+/* 1 = allow, 0 = don''t allow guest creation, -1 = don''t
allow boot */
+s8 __read_mostly opt_allow_unsafe = -1;
boolean_param("allow_unsafe", opt_allow_unsafe);
+#endif
static inline void wrmsr_amd(unsigned int index, unsigned int lo,
unsigned int hi)
@@ -496,10 +499,19 @@ static void __devinit init_amd(struct cp
clear_bit(X86_FEATURE_MWAIT, c->x86_capability);
#ifdef __x86_64__
- if (cpu_has_amd_erratum(c, AMD_ERRATUM_121) && !opt_allow_unsafe)
+ 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.\n"
"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);
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -55,6 +55,7 @@
#include <asm/traps.h>
#include <asm/nmi.h>
#include <asm/mce.h>
+#include <asm/amd.h>
#include <xen/numa.h>
#include <xen/iommu.h>
#ifdef CONFIG_COMPAT
@@ -531,6 +532,20 @@ int arch_domain_create(struct domain *d,
#else /* __x86_64__ */
+ if ( d->domain_id && !is_idle_domain(d) &&
+ cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) )
+ {
+ if ( !opt_allow_unsafe )
+ {
+ 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);
+ }
+
BUILD_BUG_ON(PDPT_L2_ENTRIES * sizeof(*d->arch.mm_perdomain_pt_pages)
!= PAGE_SIZE);
pg = alloc_domheap_page(NULL, MEMF_node(domain_to_node(d)));
--- a/xen/include/asm-x86/amd.h
+++ b/xen/include/asm-x86/amd.h
@@ -147,6 +147,8 @@ struct cpuinfo_x86;
int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...);
#ifdef __x86_64__
+extern s8 opt_allow_unsafe;
+
void fam10h_check_enable_mmcfg(void);
void check_enable_amd_mmconf_dmi(void);
#endif
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote:> Our product management wasn''t happy with the "solution" for XSA-9, and > demanded that customer systems must continue to boot. Rather than > having our and perhaps other distros carry non-trivial patches, allow > for more fine grained control (panic on boot, deny guest creation, or > merely warn) by means of a single line change.All this seems to allow is to boot but not create domU-s. Which seems a bit pointless. -- Keir> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -32,8 +32,11 @@ > static char opt_famrev[14]; > string_param("cpuid_mask_cpu", opt_famrev); > > -static bool_t opt_allow_unsafe; > +#ifdef __x86_64__ > +/* 1 = allow, 0 = don''t allow guest creation, -1 = don''t allow boot */ > +s8 __read_mostly opt_allow_unsafe = -1; > boolean_param("allow_unsafe", opt_allow_unsafe); > +#endif > > static inline void wrmsr_amd(unsigned int index, unsigned int lo, > unsigned int hi) > @@ -496,10 +499,19 @@ static void __devinit init_amd(struct cp > clear_bit(X86_FEATURE_MWAIT, c->x86_capability); > > #ifdef __x86_64__ > - if (cpu_has_amd_erratum(c, AMD_ERRATUM_121) && !opt_allow_unsafe) > + 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.\n" > "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); > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -55,6 +55,7 @@ > #include <asm/traps.h> > #include <asm/nmi.h> > #include <asm/mce.h> > +#include <asm/amd.h> > #include <xen/numa.h> > #include <xen/iommu.h> > #ifdef CONFIG_COMPAT > @@ -531,6 +532,20 @@ int arch_domain_create(struct domain *d, > > #else /* __x86_64__ */ > > + if ( d->domain_id && !is_idle_domain(d) && > + cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) ) > + { > + if ( !opt_allow_unsafe ) > + { > + 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); > + } > + > BUILD_BUG_ON(PDPT_L2_ENTRIES * sizeof(*d->arch.mm_perdomain_pt_pages) > != PAGE_SIZE); > pg = alloc_domheap_page(NULL, MEMF_node(domain_to_node(d))); > --- a/xen/include/asm-x86/amd.h > +++ b/xen/include/asm-x86/amd.h > @@ -147,6 +147,8 @@ struct cpuinfo_x86; > int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...); > > #ifdef __x86_64__ > +extern s8 opt_allow_unsafe; > + > void fam10h_check_enable_mmcfg(void); > void check_enable_amd_mmconf_dmi(void); > #endif > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> On 18.06.12 at 16:06, Keir Fraser <keir@xen.org> wrote: > On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: > >> Our product management wasn''t happy with the "solution" for XSA-9, and >> demanded that customer systems must continue to boot. Rather than >> having our and perhaps other distros carry non-trivial patches, allow >> for more fine grained control (panic on boot, deny guest creation, or >> merely warn) by means of a single line change. > > All this seems to allow is to boot but not create domU-s. Which seems a bit > pointless.I agree, but that is what our PM mandated as minimal remaining functionality (refusing to boot was not deemed acceptable). So my aim with this patch is to carry just a single line distro specific patch adjusting from the upstream default to the distro default. Carrying the full patch indefinitely just doesn''t seem desirable, but would be necessary if you''re not considering this acceptable. Jan>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/arch/x86/cpu/amd.c >> +++ b/xen/arch/x86/cpu/amd.c >> @@ -32,8 +32,11 @@ >> static char opt_famrev[14]; >> string_param("cpuid_mask_cpu", opt_famrev); >> >> -static bool_t opt_allow_unsafe; >> +#ifdef __x86_64__ >> +/* 1 = allow, 0 = don''t allow guest creation, -1 = don''t allow boot */ >> +s8 __read_mostly opt_allow_unsafe = -1; >> boolean_param("allow_unsafe", opt_allow_unsafe); >> +#endif >> >> static inline void wrmsr_amd(unsigned int index, unsigned int lo, >> unsigned int hi) >> @@ -496,10 +499,19 @@ static void __devinit init_amd(struct cp >> clear_bit(X86_FEATURE_MWAIT, c->x86_capability); >> >> #ifdef __x86_64__ >> - if (cpu_has_amd_erratum(c, AMD_ERRATUM_121) && !opt_allow_unsafe) >> + 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.\n" >> "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); >> --- a/xen/arch/x86/domain.c >> +++ b/xen/arch/x86/domain.c >> @@ -55,6 +55,7 @@ >> #include <asm/traps.h> >> #include <asm/nmi.h> >> #include <asm/mce.h> >> +#include <asm/amd.h> >> #include <xen/numa.h> >> #include <xen/iommu.h> >> #ifdef CONFIG_COMPAT >> @@ -531,6 +532,20 @@ int arch_domain_create(struct domain *d, >> >> #else /* __x86_64__ */ >> >> + if ( d->domain_id && !is_idle_domain(d) && >> + cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) ) >> + { >> + if ( !opt_allow_unsafe ) >> + { >> + 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); >> + } >> + >> BUILD_BUG_ON(PDPT_L2_ENTRIES * sizeof(*d->arch.mm_perdomain_pt_pages) >> != PAGE_SIZE); >> pg = alloc_domheap_page(NULL, MEMF_node(domain_to_node(d))); >> --- a/xen/include/asm-x86/amd.h >> +++ b/xen/include/asm-x86/amd.h >> @@ -147,6 +147,8 @@ struct cpuinfo_x86; >> int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...); >> >> #ifdef __x86_64__ >> +extern s8 opt_allow_unsafe; >> + >> void fam10h_check_enable_mmcfg(void); >> void check_enable_amd_mmconf_dmi(void); >> #endif >> >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel
On Mon, Jun 18, Keir Fraser wrote:> On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: > > > Our product management wasn''t happy with the "solution" for XSA-9, and > > demanded that customer systems must continue to boot. Rather than > > having our and perhaps other distros carry non-trivial patches, allow > > for more fine grained control (panic on boot, deny guest creation, or > > merely warn) by means of a single line change. > > All this seems to allow is to boot but not create domU-s. Which seems a bit > pointless.Refusing to boot into dom0 with no good reason is a good way to lose remote control of a system without serial console. Not funny. Fortunately I booted and tested with sles11 Xen first before ruining the box with plain xen-unstable. So, please apply this patch and remove the panic() from ./xen/arch/x86/cpu/amd.c Olaf
On 17/08/2012 16:11, "Olaf Hering" <olaf@aepfle.de> wrote:> On Mon, Jun 18, Keir Fraser wrote: > >> On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> Our product management wasn''t happy with the "solution" for XSA-9, and >>> demanded that customer systems must continue to boot. Rather than >>> having our and perhaps other distros carry non-trivial patches, allow >>> for more fine grained control (panic on boot, deny guest creation, or >>> merely warn) by means of a single line change. >> >> All this seems to allow is to boot but not create domU-s. Which seems a bit >> pointless. > > Refusing to boot into dom0 with no good reason is a good way to lose > remote control of a system without serial console. Not funny. > > Fortunately I booted and tested with sles11 Xen first before ruining the > box with plain xen-unstable. > > So, please apply this patch and remove the panic() from > ./xen/arch/x86/cpu/amd.cOkay, that''s a good argument for that patch. -- Keir> Olaf
On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote:> Our product management wasn''t happy with the "solution" for XSA-9, and > demanded that customer systems must continue to boot. Rather than > having our and perhaps other distros carry non-trivial patches, allow > for more fine grained control (panic on boot, deny guest creation, or > merely warn) by means of a single line change. > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>> --- a/xen/arch/x86/cpu/amd.c > +++ b/xen/arch/x86/cpu/amd.c > @@ -32,8 +32,11 @@ > static char opt_famrev[14]; > string_param("cpuid_mask_cpu", opt_famrev); > > -static bool_t opt_allow_unsafe; > +#ifdef __x86_64__ > +/* 1 = allow, 0 = don''t allow guest creation, -1 = don''t allow boot */ > +s8 __read_mostly opt_allow_unsafe = -1; > boolean_param("allow_unsafe", opt_allow_unsafe); > +#endif > > static inline void wrmsr_amd(unsigned int index, unsigned int lo, > unsigned int hi) > @@ -496,10 +499,19 @@ static void __devinit init_amd(struct cp > clear_bit(X86_FEATURE_MWAIT, c->x86_capability); > > #ifdef __x86_64__ > - if (cpu_has_amd_erratum(c, AMD_ERRATUM_121) && !opt_allow_unsafe) > + 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.\n" > "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); > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -55,6 +55,7 @@ > #include <asm/traps.h> > #include <asm/nmi.h> > #include <asm/mce.h> > +#include <asm/amd.h> > #include <xen/numa.h> > #include <xen/iommu.h> > #ifdef CONFIG_COMPAT > @@ -531,6 +532,20 @@ int arch_domain_create(struct domain *d, > > #else /* __x86_64__ */ > > + if ( d->domain_id && !is_idle_domain(d) && > + cpu_has_amd_erratum(&boot_cpu_data, AMD_ERRATUM_121) ) > + { > + if ( !opt_allow_unsafe ) > + { > + 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); > + } > + > BUILD_BUG_ON(PDPT_L2_ENTRIES * sizeof(*d->arch.mm_perdomain_pt_pages) > != PAGE_SIZE); > pg = alloc_domheap_page(NULL, MEMF_node(domain_to_node(d))); > --- a/xen/include/asm-x86/amd.h > +++ b/xen/include/asm-x86/amd.h > @@ -147,6 +147,8 @@ struct cpuinfo_x86; > int cpu_has_amd_erratum(const struct cpuinfo_x86 *, int, ...); > > #ifdef __x86_64__ > +extern s8 opt_allow_unsafe; > + > void fam10h_check_enable_mmcfg(void); > void check_enable_amd_mmconf_dmi(void); > #endif > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On Fri, Aug 17, Keir Fraser wrote:> On 17/08/2012 16:11, "Olaf Hering" <olaf@aepfle.de> wrote: > > > On Mon, Jun 18, Keir Fraser wrote: > > > >> On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: > >> > >>> Our product management wasn''t happy with the "solution" for XSA-9, and > >>> demanded that customer systems must continue to boot. Rather than > >>> having our and perhaps other distros carry non-trivial patches, allow > >>> for more fine grained control (panic on boot, deny guest creation, or > >>> merely warn) by means of a single line change. > >> > >> All this seems to allow is to boot but not create domU-s. Which seems a bit > >> pointless. > > > > Refusing to boot into dom0 with no good reason is a good way to lose > > remote control of a system without serial console. Not funny. > > > > Fortunately I booted and tested with sles11 Xen first before ruining the > > box with plain xen-unstable. > > > > So, please apply this patch and remove the panic() from > > ./xen/arch/x86/cpu/amd.c > > Okay, that''s a good argument for that patch.Oh, now that the context was posted again: With the patch the box would still panic per default. Leaving it zero to refuse guest creation looks like a sensible default. Olaf
>>> On 17.08.12 at 17:56, Olaf Hering <olaf@aepfle.de> wrote: > On Fri, Aug 17, Keir Fraser wrote: > >> On 17/08/2012 16:11, "Olaf Hering" <olaf@aepfle.de> wrote: >> >> > On Mon, Jun 18, Keir Fraser wrote: >> > >> >> On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: >> >> >> >>> Our product management wasn''t happy with the "solution" for XSA-9, and >> >>> demanded that customer systems must continue to boot. Rather than >> >>> having our and perhaps other distros carry non-trivial patches, allow >> >>> for more fine grained control (panic on boot, deny guest creation, or >> >>> merely warn) by means of a single line change. >> >> >> >> All this seems to allow is to boot but not create domU-s. Which seems a bit >> >> pointless. >> > >> > Refusing to boot into dom0 with no good reason is a good way to lose >> > remote control of a system without serial console. Not funny. >> > >> > Fortunately I booted and tested with sles11 Xen first before ruining the >> > box with plain xen-unstable. >> > >> > So, please apply this patch and remove the panic() from >> > ./xen/arch/x86/cpu/amd.c >> >> Okay, that''s a good argument for that patch. > > Oh, now that the context was posted again: > With the patch the box would still panic per default. Leaving it zero to > refuse guest creation looks like a sensible default.Keir, should I change the default then before committing? Jan
On 17/08/2012 17:28, "Jan Beulich" <JBeulich@suse.com> wrote:>>>> On 17.08.12 at 17:56, Olaf Hering <olaf@aepfle.de> wrote: >> On Fri, Aug 17, Keir Fraser wrote: >> >>> On 17/08/2012 16:11, "Olaf Hering" <olaf@aepfle.de> wrote: >>> >>>> On Mon, Jun 18, Keir Fraser wrote: >>>> >>>>> On 13/06/2012 11:04, "Jan Beulich" <JBeulich@suse.com> wrote: >>>>> >>>>>> Our product management wasn''t happy with the "solution" for XSA-9, and >>>>>> demanded that customer systems must continue to boot. Rather than >>>>>> having our and perhaps other distros carry non-trivial patches, allow >>>>>> for more fine grained control (panic on boot, deny guest creation, or >>>>>> merely warn) by means of a single line change. >>>>> >>>>> All this seems to allow is to boot but not create domU-s. Which seems a >>>>> bit >>>>> pointless. >>>> >>>> Refusing to boot into dom0 with no good reason is a good way to lose >>>> remote control of a system without serial console. Not funny. >>>> >>>> Fortunately I booted and tested with sles11 Xen first before ruining the >>>> box with plain xen-unstable. >>>> >>>> So, please apply this patch and remove the panic() from >>>> ./xen/arch/x86/cpu/amd.c >>> >>> Okay, that''s a good argument for that patch. >> >> Oh, now that the context was posted again: >> With the patch the box would still panic per default. Leaving it zero to >> refuse guest creation looks like a sensible default. > > Keir, should I change the default then before committing?Yes please. -- Keir> Jan >