Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On 15/06/18 23:10, Jiri Kosina wrote: > On Fri, 15 Jun 2018, Juergen Gross wrote: > >> Like it is possible to switch off PTI in the kernel it is possible to do >> the same with XPTI in the hypervisor (it is even possible to disable >> XPTI for dom0 only). >> >> In case XPTI is disabled for the currently running system it is possible >> to make use of Meltdown in user programs to read arbitrary physical host >> memory (i.e. attacking the hypervisor) and this includes the own systems >> kernel memory. >> >> So telling a user the system isn't vulnerable regarding Meltdown when >> running as 64-bit pv-guest might not be the truth. > > Ok, what a mess. > > As I don't think it'd be wise to try to let guest kernel figure out > whether host has XPTI, I'd suggest at least making the message somehow > more informative. Something like > > + if (hypervisor_is_type(X86_HYPER_XEN_PV)) > + return sprintf(buf, "Unknown (XEN PV detected, > hypervisor mitigation required\n"); > > perhaps? Works for me. Juergen
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On Fri, 15 Jun 2018, Juergen Gross wrote: > Like it is possible to switch off PTI in the kernel it is possible to do > the same with XPTI in the hypervisor (it is even possible to disable > XPTI for dom0 only). > > In case XPTI is disabled for the currently running system it is possible > to make use of Meltdown in user programs to read arbitrary physical host > memory (i.e. attacking the hypervisor) and this includes the own systems > kernel memory. > > So telling a user the system isn't vulnerable regarding Meltdown when > running as 64-bit pv-guest might not be the truth. Ok, what a mess. As I don't think it'd be wise to try to let guest kernel figure out whether host has XPTI, I'd suggest at least making the message somehow more informative. Something like + if (hypervisor_is_type(X86_HYPER_XEN_PV)) + return sprintf(buf, "Unknown (XEN PV detected, hypervisor mitigation required\n"); perhaps? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On 15/06/18 08:39, Jiri Kosina wrote: > On Fri, 15 Jun 2018, Juergen Gross wrote: > >> Why? PTI has to be disabled in PV guests as it can't work there due to >> missing paravirtualization of the PTI feature (mov to/from %cr3). >> >> The Xen meltdown mitigation ("XPTI") for 64-bit pv guests is primarily >> securing the hypervisor against meltdown attacks of the guest. The guest >> itself can't do anything in this regard in 64-bit mode, as user and >> kernel code are already using different %cr3 values even without PTI. > > That I know. Then I am probably dense today, but could you please again > explain what you meant by this in your first reply: > > "This is wrong for [ ... ] for 64-bit, too, in case the mitigation is >disabled at hypervisor level." > Like it is possible to switch off PTI in the kernel it is possible to do the same with XPTI in the hypervisor (it is even possible to disable XPTI for dom0 only). In case XPTI is disabled for the currently running system it is possible to make use of Meltdown in user programs to read arbitrary physical host memory (i.e. attacking the hypervisor) and this includes the own systems kernel memory. So telling a user the system isn't vulnerable regarding Meltdown when running as 64-bit pv-guest might not be the truth. Juergen
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On Fri, 15 Jun 2018, Juergen Gross wrote: > Why? PTI has to be disabled in PV guests as it can't work there due to > missing paravirtualization of the PTI feature (mov to/from %cr3). > > The Xen meltdown mitigation ("XPTI") for 64-bit pv guests is primarily > securing the hypervisor against meltdown attacks of the guest. The guest > itself can't do anything in this regard in 64-bit mode, as user and > kernel code are already using different %cr3 values even without PTI. That I know. Then I am probably dense today, but could you please again explain what you meant by this in your first reply: "This is wrong for [ ... ] for 64-bit, too, in case the mitigation is disabled at hypervisor level." -- Jiri Kosina SUSE Labs
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On 15/06/18 08:16, Jiri Kosina wrote: > On Fri, 15 Jun 2018, Juergen Gross wrote: > wrong for 64-bit, too, in case the mitigation is disabled at hypervisor level. >>> >>> If that is indeed possible (is it?), then the check we have in >>> pti_check_boottime_disable() is wrong as well. >> >> No, it isn't. PTI for 32-bit kernels isn't paravirtualized, so it has to >> be disabled. > > I was talking about this "mitigation disabled at Xen hypervisor level for > 64-bit" situation though. > Why? PTI has to be disabled in PV guests as it can't work there due to missing paravirtualization of the PTI feature (mov to/from %cr3). The Xen meltdown mitigation ("XPTI") for 64-bit pv guests is primarily securing the hypervisor against meltdown attacks of the guest. The guest itself can't do anything in this regard in 64-bit mode, as user and kernel code are already using different %cr3 values even without PTI. Juergen
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On Fri, 15 Jun 2018, Juergen Gross wrote: > >> wrong for 64-bit, too, in case the mitigation is disabled at hypervisor > >> level. > > > > If that is indeed possible (is it?), then the check we have in > > pti_check_boottime_disable() is wrong as well. > > No, it isn't. PTI for 32-bit kernels isn't paravirtualized, so it has to > be disabled. I was talking about this "mitigation disabled at Xen hypervisor level for 64-bit" situation though. -- Jiri Kosina SUSE Labs
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On 15/06/18 08:04, Jiri Kosina wrote: > On Fri, 15 Jun 2018, Juergen Gross wrote: > >> wrong for 64-bit, too, in case the mitigation is disabled at hypervisor >> level. > > If that is indeed possible (is it?), then the check we have in > pti_check_boottime_disable() is wrong as well. No, it isn't. PTI for 32-bit kernels isn't paravirtualized, so it has to be disabled. > >> So the test should be done only for CONFIG_X86_64 > > Fair enough. > >> and the returned string should be e.g. "Mitigation: XEN". > > Well, perhaps; it'd confuse all the scripts that are checking whether > system is fully secured or not by parsing sysfs files ... but that's > mostly their problem. Right. And I suppose those scripts are fairly new. :-) Juergen
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On Fri, 15 Jun 2018, Juergen Gross wrote: > wrong for 64-bit, too, in case the mitigation is disabled at hypervisor > level. If that is indeed possible (is it?), then the check we have in pti_check_boottime_disable() is wrong as well. > So the test should be done only for CONFIG_X86_64 Fair enough. > and the returned string should be e.g. "Mitigation: XEN". Well, perhaps; it'd confuse all the scripts that are checking whether system is fully secured or not by parsing sysfs files ... but that's mostly their problem. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] x86/pti: don't report XenPV as vulnerable
On 15/06/18 00:32, Jiri Kosina wrote: > From: Jiri Kosina > > Xen PV domain is not by design affected by meltdown as it's enforcing > split CR3 itself. Let's not report such systems as "Vulnerable" in sysfs > (we're also already forcing PTI to off in X86_HYPER_XEN_PV cases) > > Reported-and-tested-by: Mike Latimer > Signed-off-by: Jiri Kosina > --- > > I originally wanted to just not set X86_BUG_CPU_MELTDOWN in > cpu_set_bug_bits() in the first place, but that has two issues: > > - cpu_set_bug_bits() gets invoked from early_identify_cpu() before > init_hypervisor_platform() had a chance to run, and therefore the > hypervisor type check doesn't work there > > - it'd actually be inaccurate; the CPU *does* have the bug at the end > of the day (so it's properly kept being reported in cpuinfo), it's > "just a setup matter" that we don't need any addtional mitigation to > be applied by the kernel > > So let's not overcomplicate it. > > arch/x86/kernel/cpu/bugs.c |4 > 1 file changed, 4 insertions(+) > > --- a/arch/x86/kernel/cpu/bugs.c > +++ b/arch/x86/kernel/cpu/bugs.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > static void __init spectre_v2_select_mitigation(void); > static void __init ssb_select_mitigation(void); > @@ -685,6 +686,9 @@ static ssize_t cpu_show_common(struct de > if (boot_cpu_has(X86_FEATURE_PTI)) > return sprintf(buf, "Mitigation: PTI\n"); > > + if (hypervisor_is_type(X86_HYPER_XEN_PV)) > + return sprintf(buf, "Not affected\n"); I don't like this. This is wrong for 32-bit guests and maybe wrong for 64-bit, too, in case the mitigation is disabled at hypervisor level. So the test should be done only for CONFIG_X86_64 and the returned string should be e.g. "Mitigation: XEN". Juergen
[PATCH] x86/pti: don't report XenPV as vulnerable
From: Jiri Kosina Xen PV domain is not by design affected by meltdown as it's enforcing split CR3 itself. Let's not report such systems as "Vulnerable" in sysfs (we're also already forcing PTI to off in X86_HYPER_XEN_PV cases) Reported-and-tested-by: Mike Latimer Signed-off-by: Jiri Kosina --- I originally wanted to just not set X86_BUG_CPU_MELTDOWN in cpu_set_bug_bits() in the first place, but that has two issues: - cpu_set_bug_bits() gets invoked from early_identify_cpu() before init_hypervisor_platform() had a chance to run, and therefore the hypervisor type check doesn't work there - it'd actually be inaccurate; the CPU *does* have the bug at the end of the day (so it's properly kept being reported in cpuinfo), it's "just a setup matter" that we don't need any addtional mitigation to be applied by the kernel So let's not overcomplicate it. arch/x86/kernel/cpu/bugs.c |4 1 file changed, 4 insertions(+) --- a/arch/x86/kernel/cpu/bugs.c +++ b/arch/x86/kernel/cpu/bugs.c @@ -26,6 +26,7 @@ #include #include #include +#include static void __init spectre_v2_select_mitigation(void); static void __init ssb_select_mitigation(void); @@ -685,6 +686,9 @@ static ssize_t cpu_show_common(struct de if (boot_cpu_has(X86_FEATURE_PTI)) return sprintf(buf, "Mitigation: PTI\n"); + if (hypervisor_is_type(X86_HYPER_XEN_PV)) + return sprintf(buf, "Not affected\n"); + break; case X86_BUG_SPECTRE_V1: -- Jiri Kosina SUSE Labs