[PATCH] x86/pti: don't report XenPV as vulnerable

[PATCH] x86/pti: don't report XenPV as vulnerable

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

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 <mlatimer@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---

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 <asm/pgtable.h>
 #include <asm/set_memory.h>
 #include <asm/intel-family.h>
+#include <asm/hypervisor.h>
 
 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

Re: [PATCH] x86/pti: don't report XenPV as vulnerable

[Juergen Gross]

On 15/06/18 00:32, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> 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 <mlatimer@suse.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> 
> 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 <asm/pgtable.h>
>  #include <asm/set_memory.h>
>  #include <asm/intel-family.h>
> +#include <asm/hypervisor.h>
>  
>  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

Re: [PATCH] x86/pti: don't report XenPV as vulnerable

[Jiri Kosina]

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

[Juergen Gross]

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

[Jiri Kosina]

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

[Juergen Gross]

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

[Jiri Kosina]

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

[Juergen Gross]

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

[Jiri Kosina]

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

[Juergen Gross]

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

[PATCH v2] x86/pti: don't report XenPV as vulnerable

[Jiri Kosina]

From: Jiri Kosina <jkosina@suse.cz>

Xen PV domain kernel 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); the security of the system ultimately depends on presence of 
mitigation in Hypervisor, which can't be easily detected from DomU; let's 
report that.

Reported-and-tested-by: Mike Latimer <mlatimer@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---

v1 -> v2: reporting "Not affected" is not really correct, as it depends on
	  the presence of mitigation in the hypervisor

 arch/x86/kernel/cpu/bugs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index cd0fda1fff6d..57638396a254 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -27,6 +27,7 @@
 #include <asm/pgtable.h>
 #include <asm/set_memory.h>
 #include <asm/intel-family.h>
+#include <asm/hypervisor.h>
 
 static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
@@ -664,6 +665,10 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 		if (boot_cpu_has(X86_FEATURE_PTI))
 			return sprintf(buf, "Mitigation: PTI\n");
 
+		if (hypervisor_is_type(X86_HYPER_XEN_PV))
+			return sprintf(buf, "Unknown (XEN PV detected, hypervisor "
+					    "mitigation required)\n");
+
 		break;
 
 	case X86_BUG_SPECTRE_V1:
-- 
2.12.3

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v2] x86/pti: don't report XenPV as vulnerable

[Juergen Gross]

On 18/06/18 09:59, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
> 
> Xen PV domain kernel 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); the security of the system ultimately depends on presence of 
> mitigation in Hypervisor, which can't be easily detected from DomU; let's 
> report that.
> 
> Reported-and-tested-by: Mike Latimer <mlatimer@suse.com>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>

Acked-by: Juergen Gross <jgross@suse.com>


Juergen

[tip:x86/pti] x86/pti: Don't report XenPV as vulnerable

[tip-bot for Jiri Kosina]

Commit-ID:  66aa6b5cbc359331fc054e96bb49e9502bc0b1d9
Gitweb:     https://git.kernel.org/tip/66aa6b5cbc359331fc054e96bb49e9502bc0b1d9
Author:     Jiri Kosina <jkosina@suse.cz>
AuthorDate: Mon, 18 Jun 2018 09:59:54 +0200
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Tue, 19 Jun 2018 09:58:22 +0200

x86/pti: Don't report XenPV as vulnerable

Xen PV domain kernel 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);
the security of the system ultimately depends on presence of mitigation in
the Hypervisor, which can't be easily detected from DomU; let's report
that.

Reported-and-tested-by: Mike Latimer <mlatimer@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Juergen Gross <jgross@suse.com>
Cc: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/nycvar.YFH.7.76.1806180959080.6203@cbobk.fhfr.pm

---
 arch/x86/kernel/cpu/bugs.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index cd0fda1fff6d..57638396a254 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -27,6 +27,7 @@
 #include <asm/pgtable.h>
 #include <asm/set_memory.h>
 #include <asm/intel-family.h>
+#include <asm/hypervisor.h>
 
 static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
@@ -664,6 +665,10 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 		if (boot_cpu_has(X86_FEATURE_PTI))
 			return sprintf(buf, "Mitigation: PTI\n");
 
+		if (hypervisor_is_type(X86_HYPER_XEN_PV))
+			return sprintf(buf, "Unknown (XEN PV detected, hypervisor "
+					    "mitigation required)\n");
+
 		break;
 
 	case X86_BUG_SPECTRE_V1:

[tip:x86/pti] x86/pti: Don't report XenPV as vulnerable

[tip-bot for Jiri Kosina]

Commit-ID:  6cb2b08ff92460290979de4be91363e5d1b6cec1
Gitweb:     https://git.kernel.org/tip/6cb2b08ff92460290979de4be91363e5d1b6cec1
Author:     Jiri Kosina <jkosina@suse.cz>
AuthorDate: Mon, 18 Jun 2018 09:59:54 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 21 Jun 2018 14:14:52 +0200

x86/pti: Don't report XenPV as vulnerable

Xen PV domain kernel 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);
the security of the system ultimately depends on presence of mitigation in
the Hypervisor, which can't be easily detected from DomU; let's report
that.

Reported-and-tested-by: Mike Latimer <mlatimer@suse.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Acked-by: Juergen Gross <jgross@suse.com>
Cc: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/nycvar.YFH.7.76.1806180959080.6203@cbobk.fhfr.pm
[ Merge the user-visible string into a single line. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/cpu/bugs.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index cd0fda1fff6d..404df26b7de8 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -27,6 +27,7 @@
 #include <asm/pgtable.h>
 #include <asm/set_memory.h>
 #include <asm/intel-family.h>
+#include <asm/hypervisor.h>
 
 static void __init spectre_v2_select_mitigation(void);
 static void __init ssb_select_mitigation(void);
@@ -664,6 +665,9 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
 		if (boot_cpu_has(X86_FEATURE_PTI))
 			return sprintf(buf, "Mitigation: PTI\n");
 
+		if (hypervisor_is_type(X86_HYPER_XEN_PV))
+			return sprintf(buf, "Unknown (XEN PV detected, hypervisor mitigation required)\n");
+
 		break;
 
 	case X86_BUG_SPECTRE_V1: