mitigate against CVE-2017-5715 by flushing the return stack buffer (RSB) upon returning from the guest
ClosedPublic
Actions

Other option to have single vmx_exit_guest but use sysctl to enable/disable RSB flush, something like this[its for SVM but we can do similar for vmx].

Index: sys/amd64/vmm/amd/svm.c
===================================================================
--- sys/amd64/vmm/amd/svm.c	(revision 328628)
+++ sys/amd64/vmm/amd/svm.c	(working copy)
@@ -115,6 +115,13 @@
 SYSCTL_UINT(_hw_vmm_svm, OID_AUTO, num_asids, CTLFLAG_RDTUN, &nasid, 0,
     "Number of ASIDs supported by this processor");
 
+int flush_rsb_enable = 1;
+SYSCTL_UINT(_hw_vmm_svm, OID_AUTO, flus_rsb_enable, CTLFLAG_RDTUN, &flush_rsb_enable, 0,
+    "Enable RSB flush");
+
+int flush_rsb_count = 0;
+SYSCTL_UINT(_hw_vmm_svm, OID_AUTO, flus_rsb_count, CTLFLAG_RDTUN, &flush_rsb_count, 0,
+    "RSB flush count");
 /* Current ASID generation for each host cpu */
 static struct asid asid[MAXCPU];
 
Index: sys/amd64/vmm/amd/svm_genassym.c
===================================================================
--- sys/amd64/vmm/amd/svm_genassym.c	(revision 328628)
+++ sys/amd64/vmm/amd/svm_genassym.c	(working copy)
@@ -33,6 +33,8 @@
 
 #include "svm.h"
 
+extern int flush_rsb_enable;
+
 ASSYM(SCTX_RBX, offsetof(struct svm_regctx, sctx_rbx));
 ASSYM(SCTX_RCX, offsetof(struct svm_regctx, sctx_rcx));
 ASSYM(SCTX_RBP, offsetof(struct svm_regctx, sctx_rbp));
Index: sys/amd64/vmm/amd/svm_support.S
===================================================================
--- sys/amd64/vmm/amd/svm_support.S	(revision 328628)
+++ sys/amd64/vmm/amd/svm_support.S	(working copy)
@@ -41,6 +41,8 @@
 #define	VMRUN	.byte 0x0f, 0x01, 0xd8
 #define	VMSAVE	.byte 0x0f, 0x01, 0xdb
 
+.extern flush_rsb_enable
+.extern flush_rsb_count
 /*
  * svm_launch(uint64_t vmcb, struct svm_regctx *gctx, struct pcpu *pcpu)
  * %rdi: physical address of VMCB
@@ -113,8 +115,29 @@
 	movq %rdi, SCTX_RDI(%rax)
 	movq %rsi, SCTX_RSI(%rax)
 
+        cmpq $1, flush_rsb_enable
+        jne no_rsb_flush
+        incq flush_rsb_count
+
+        /*
+         * To prevent malicious branch target predictions from
+         * affecting the host, overwrite all entries in the RSB upon
+         * exiting a guest.
+         */
+        mov $16, %ecx        /* 16 iterations, two calls per loop */
+        mov %rsp, %rax
+0:        call 2f                /* create an RSB entry. */
+1:        pause
+        call 1b                /* capture rogue speculation. */
+2:        call 2f                /* create an RSB entry. */
+1:        pause
+        call 1b                /* capture rogue speculation. */
+2:        sub $1, %ecx
+        jnz 0b
+        mov %rax, %rsp
+ 
 	/* Restore host state */
-	pop %r15
+no_rsb_flush:	pop %r15
 	pop %r14
 	pop %r13
 	pop %r12
$

Closed by commit rS329162: Provide further mitigation against CVE-2017-5715 by flushing the (authored by tychon). · Explain WhyFeb 12 2018, 2:45 PM

This revision was automatically updated to reflect the committed changes.

Herald added a subscriber: imp. · View Herald TranscriptFeb 12 2018, 2:45 PM

While a conditional branch dependent on global variable isn't quite the same as an indirect branch dependent on a global variable, it would seem the opportunity still exists to evict the cache line containing the global variable to give the CPU some time to speculatively execute in the neighborhood of the branch. What I've seen in other implementation is the placement of the rsb flushing code before any branches. Perhaps that's overly conservative but I think we should follow that approach too.

It's for that reason that I duplicated the entire guest exit path for intel and inserted the flush routines into one giving us a standard exit with differentiated returns. The analogue for amd could involve a common exit path with separate enter/flush paths and a knob to switch between them. Alas, without a way to test them I opted to go there.

I'm going to commit what I have now; that doesn't preclude enhancing the amd version in a follow-on commit.