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

2018-06-16 Thread 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


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

2018-06-16 Thread 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


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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-15 Thread 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

2018-06-14 Thread Juergen Gross
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



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

2018-06-14 Thread Juergen Gross
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

2018-06-14 Thread Jiri Kosina
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



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

2018-06-14 Thread Jiri Kosina
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