(partial) Spectre v2 mitigation without on Skylake IBRS

(partial) Spectre v2 mitigation without on Skylake IBRS

[Jan Beulich]

All,

if running PV Linux on older Xen (4.5 and earlier) is relevant, it may be
necessary to use a mechanism other than IBRS to mitigate Spectre v2
on Skylake. That is because the new MSR value can't be migrated
prior to migration v2. Of course one option would be to retrofit some
mechanism into newer Xen versions that makes them accept whatever
extension to e.g. struct hvm_hw_cpu one might want to invent for
the older Xen versions. But that doesn't seem very desirable.

Hence I've been considering a kernel side approach as alternative.
To recap, iirc the reason retpoline isn't enough on Skylake is that RET
may consult the BTB upon RSB underflow. All other aspects can be
dealt with using the existing combination of retpoline and RSB stuffing.
The idea is to use a retpoline for RET as well:

RET_repoline:
	call	2f
1:
	lfence
	jmp	1b
2:
	lea	8(%rsp), %rsp
	ret

(32-bit version would be quite similar). This would mostly avoid the
risk of RSB underflows; the case of async events - SMI being the
worst - in the middle of the thunk of course wouldn't be possible to
deal with, but I think that might be acceptable, as an attacker can't
control their arrival.

Obviously this isn't going to be easy without compiler support, yet
I'm unconvinced we could talk the compiler folks into adding yet
another option to support such a mode, the more that it would be
a partial mitigation only. Hence the other way of achieving this
would need considering - inject a "ret" macro into the assembler
output produced by the compiler, converting every ret to a branch
to the thunk. This, in turn, may be a hard sell to the x86
maintainers. Otoh, having done such for other purposes already
(experimental only, i.e. never posted publicly anywhere) I think I
have all the building pieces ready for re-use.

Opinions appreciated,
Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: (partial) Spectre v2 mitigation without on Skylake IBRS

[Juergen Gross]

On 26/02/18 10:44, Jan Beulich wrote:
> All,
> 
> if running PV Linux on older Xen (4.5 and earlier) is relevant, it may be
> necessary to use a mechanism other than IBRS to mitigate Spectre v2
> on Skylake. That is because the new MSR value can't be migrated
> prior to migration v2. Of course one option would be to retrofit some
> mechanism into newer Xen versions that makes them accept whatever
> extension to e.g. struct hvm_hw_cpu one might want to invent for
> the older Xen versions. But that doesn't seem very desirable.

Can you please elaborate a little bit more what the real problem is?

I _think_ you are referring to the problem that a pv kernel would want
to use IBRS for mitigation of Spectre V2 and after a migration that
setting would be lost.

If this is the case I believe the easiest solution would be to let the
kernel set the MSR again after leaving suspended state. suspend/resume
require hooks in pv kernels after all.


Juergen


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: (partial) Spectre v2 mitigation without on Skylake IBRS

[Jan Beulich]

>>> On 26.02.18 at 11:18, <jgross@suse.com> wrote:
> On 26/02/18 10:44, Jan Beulich wrote:
>> if running PV Linux on older Xen (4.5 and earlier) is relevant, it may be
>> necessary to use a mechanism other than IBRS to mitigate Spectre v2
>> on Skylake. That is because the new MSR value can't be migrated
>> prior to migration v2. Of course one option would be to retrofit some
>> mechanism into newer Xen versions that makes them accept whatever
>> extension to e.g. struct hvm_hw_cpu one might want to invent for
>> the older Xen versions. But that doesn't seem very desirable.
> 
> Can you please elaborate a little bit more what the real problem is?
> 
> I _think_ you are referring to the problem that a pv kernel would want
> to use IBRS for mitigation of Spectre V2 and after a migration that
> setting would be lost.

"Lost" is the wrong term imo: A hypervisor that's been patched for
Spectre v2 (and that's a prereq anyway, because we want the
kernel to use IBPB, which utilizes an MSR that doesn't need
migrating) should at least do _something_ with the MSR (when it's
non-zero). The most natural thing (imo) is to make those older
hypervisors support XEN_DOMCTL_{get,set}_vcpu_msrs. That in
turn calls for the tool stack to gain the check that Andrew had
added to libxc in db24f7f012 ("libxc: use an explicit check for PV
MSRs in xc_domain_save() "), causing migration to fail when the
MSR is non-zero on any of the guest's vCPU-s.

> If this is the case I believe the easiest solution would be to let the
> kernel set the MSR again after leaving suspended state. suspend/resume
> require hooks in pv kernels after all.

Hmm, this could be leveraged irrespective of what I've written
above - the kernel could then also clear the MSR during suspend,
thus allowing the check in libxc to pass.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: (partial) Spectre v2 mitigation without on Skylake IBRS

[Juergen Gross]

[-- Attachment #1: Type: text/plain, Size: 1936 bytes --]

On 26/02/18 11:49, Jan Beulich wrote:
>>>> On 26.02.18 at 11:18, <jgross@suse.com> wrote:
>> On 26/02/18 10:44, Jan Beulich wrote:
>>> if running PV Linux on older Xen (4.5 and earlier) is relevant, it may be
>>> necessary to use a mechanism other than IBRS to mitigate Spectre v2
>>> on Skylake. That is because the new MSR value can't be migrated
>>> prior to migration v2. Of course one option would be to retrofit some
>>> mechanism into newer Xen versions that makes them accept whatever
>>> extension to e.g. struct hvm_hw_cpu one might want to invent for
>>> the older Xen versions. But that doesn't seem very desirable.
>>
>> Can you please elaborate a little bit more what the real problem is?
>>
>> I _think_ you are referring to the problem that a pv kernel would want
>> to use IBRS for mitigation of Spectre V2 and after a migration that
>> setting would be lost.
> 
> "Lost" is the wrong term imo: A hypervisor that's been patched for
> Spectre v2 (and that's a prereq anyway, because we want the
> kernel to use IBPB, which utilizes an MSR that doesn't need
> migrating) should at least do _something_ with the MSR (when it's
> non-zero). The most natural thing (imo) is to make those older
> hypervisors support XEN_DOMCTL_{get,set}_vcpu_msrs. That in
> turn calls for the tool stack to gain the check that Andrew had
> added to libxc in db24f7f012 ("libxc: use an explicit check for PV
> MSRs in xc_domain_save() "), causing migration to fail when the
> MSR is non-zero on any of the guest's vCPU-s.
> 
>> If this is the case I believe the easiest solution would be to let the
>> kernel set the MSR again after leaving suspended state. suspend/resume
>> require hooks in pv kernels after all.
> 
> Hmm, this could be leveraged irrespective of what I've written
> above - the kernel could then also clear the MSR during suspend,
> thus allowing the check in libxc to pass.

Something like the attached patch?


Juergen


[-- Attachment #2: 0001-x86-xen-zero-MSR_IA32_SPEC_CTRL-before-suspend.patch --]
[-- Type: text/x-patch, Size: 1502 bytes --]

>From 5a03c1e0a21f3a5a3f2228e5df7150d3f3be6e1f Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Mon, 26 Feb 2018 13:10:55 +0100
Subject: [PATCH] x86/xen: zero MSR_IA32_SPEC_CTRL before suspend

Older Xen versions (before 4.5) might have problems migrating pv guests
with MSR_IA32_SPEC_CTRL having a non-zero value. So before suspending
zero that MSR and restore it after being resumed.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 arch/x86/xen/suspend.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index d9f96cc5d743..2eb3069fd413 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -15,6 +15,8 @@
 #include "mmu.h"
 #include "pmu.h"
 
+static DEFINE_PER_CPU(u64, spec_ctrl);
+
 void xen_arch_pre_suspend(void)
 {
 	xen_save_time_memory_area();
@@ -35,6 +37,9 @@ void xen_arch_post_suspend(int cancelled)
 
 static void xen_vcpu_notify_restore(void *data)
 {
+	if (xen_pv_domain())
+		wrmsrl(MSR_IA32_SPEC_CTRL, this_cpu_read(spec_ctrl));
+
 	/* Boot processor notified via generic timekeeping_resume() */
 	if (smp_processor_id() == 0)
 		return;
@@ -44,7 +49,15 @@ static void xen_vcpu_notify_restore(void *data)
 
 static void xen_vcpu_notify_suspend(void *data)
 {
+	u64 tmp;
+
 	tick_suspend_local();
+
+	if (xen_pv_domain()) {
+		rdmsrl(MSR_IA32_SPEC_CTRL, tmp);
+		this_cpu_write(spec_ctrl, tmp);
+		wrmsrl(MSR_IA32_SPEC_CTRL, 0);
+	}
 }
 
 void xen_arch_resume(void)
-- 
2.13.6


[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: (partial) Spectre v2 mitigation without on Skylake IBRS

[Jan Beulich]

>>> On 26.02.18 at 13:36, <jgross@suse.com> wrote:
> On 26/02/18 11:49, Jan Beulich wrote:
>>>>> On 26.02.18 at 11:18, <jgross@suse.com> wrote:
>>> If this is the case I believe the easiest solution would be to let the
>>> kernel set the MSR again after leaving suspended state. suspend/resume
>>> require hooks in pv kernels after all.
>> 
>> Hmm, this could be leveraged irrespective of what I've written
>> above - the kernel could then also clear the MSR during suspend,
>> thus allowing the check in libxc to pass.
> 
> Something like the attached patch?

With proper checking added of whether the MSR actually exists,
yes, I think so. Will need to see how this can be converted to
something that works on the old XenoLinux trees.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: (partial) Spectre v2 mitigation without on Skylake IBRS

[Juergen Gross]

On 26/02/18 14:11, Jan Beulich wrote:
>>>> On 26.02.18 at 13:36, <jgross@suse.com> wrote:
>> On 26/02/18 11:49, Jan Beulich wrote:
>>>>>> On 26.02.18 at 11:18, <jgross@suse.com> wrote:
>>>> If this is the case I believe the easiest solution would be to let the
>>>> kernel set the MSR again after leaving suspended state. suspend/resume
>>>> require hooks in pv kernels after all.
>>>
>>> Hmm, this could be leveraged irrespective of what I've written
>>> above - the kernel could then also clear the MSR during suspend,
>>> thus allowing the check in libxc to pass.
>>
>> Something like the attached patch?
> 
> With proper checking added of whether the MSR actually exists,
> yes, I think so. Will need to see how this can be converted to
> something that works on the old XenoLinux trees.

Okay, will post the (modified) patch to lkml.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel