x86: nVMX: add new test for vmread/vmwrite flags preservation

This commit adds new unit tests for commit a4d956b93904 ("KVM: nVMX:
vmread should not set rflags to specify success in case of #PF")

The two new tests force a vmread and a vmwrite on an unmapped address to cause a #PF and verify that the low byte of %rflags is preserved and that %rip is not advanced. The commit fixed a bug in vmread, but we include a test for vmwrite as well for completeness.

Before the aforementioned commit, the ALU flags would be incorrectly cleared and %rip would be advanced (for vmread).

Signed-off-by Simon Smith <[email protected]>

Reviewed-by Jim Mattson <[email protected]>

Reviewed-by Peter Shier <[email protected]>

Reviewed-by Krish Sadhukhan <[email protected]>

Reviewed-by Oliver Upton <[email protected]>

---
 x86/vmx.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

x86/vmx.c +140 -1

index 4c47eec1a1597..cbe68761894d4 100644
@@ -32,6 +32,7 @@
 #include "processor.h"
 #include "alloc_page.h"
 #include "vm.h"
+#include "vmalloc.h"
 #include "desc.h"
 #include "vmx.h"
 #include "msr.h"

@@ -387,6 +388,141 @@ static void test_vmwrite_vmread(void)
        free_page(vmcs);
 }
 
+ulong finish_fault;
+u8 sentinel;
+bool handler_called;
+
+static void pf_handler(struct ex_regs *regs)
+{
+       /*
+        * check that RIP was not improperly advanced and that the
+        * flags value was preserved.
+        */
+       report(regs->rip < finish_fault, "RIP has not been advanced!");
+       report(((u8)regs->rflags == ((sentinel | 2) & 0xd7)),
+              "The low byte of RFLAGS was preserved!");
+       regs->rip = finish_fault;
+       handler_called = true;
+
+}
+
+static void prep_flags_test_env(void **vpage, struct vmcs **vmcs, handler *old)
+{
+       /*
+        * get an unbacked address that will cause a #PF
+        */
+       *vpage = alloc_vpage();
+
+       /*
+        * set up VMCS so we have something to read from
+        */
+       *vmcs = alloc_page();
+
+       memset(*vmcs, 0, PAGE_SIZE);
+       (*vmcs)->hdr.revision_id = basic.revision;
+       assert(!vmcs_clear(*vmcs));
+       assert(!make_vmcs_current(*vmcs));
+
+       *old = handle_exception(PF_VECTOR, &pf_handler);
+}
+
+static void test_read_sentinel(void)
+{
+       void *vpage;
+       struct vmcs *vmcs;
+       handler old;
+
+       prep_flags_test_env(&vpage, &vmcs, &old);
+
+       /*
+        * set the proper label
+        */
+       extern char finish_read_fault;
+
+       finish_fault = (ulong)&finish_read_fault;
+
+       /*
+        * execute the vmread instruction that will cause a #PF
+        */
+       handler_called = false;
+       asm volatile ("movb %[byte], %%ah\n\t"
+                     "sahf\n\t"
+                     "vmread %[enc], %[val]; finish_read_fault:"
+                     : [val] "=m" (*(u64 *)vpage)
+                     : [byte] "Krm" (sentinel),
+                     [enc] "r" ((u64)GUEST_SEL_SS)
+                     : "cc", "ah");
+       report(handler_called, "The #PF handler was invoked");
+
+       /*
+        * restore the old #PF handler
+        */
+       handle_exception(PF_VECTOR, old);
+}
+
+static void test_vmread_flags_touch(void)
+{
+       /*
+        * set up the sentinel value in the flags register. we
+        * choose these two values because they candy-stripe
+        * the 5 flags that sahf sets.
+        */
+       sentinel = 0x91;
+       test_read_sentinel();
+
+       sentinel = 0x45;
+       test_read_sentinel();
+}
+
+static void test_write_sentinel(void)
+{
+       void *vpage;
+       struct vmcs *vmcs;
+       handler old;
+
+       prep_flags_test_env(&vpage, &vmcs, &old);
+
+       /*
+        * set the proper label
+        */
+       extern char finish_write_fault;
+
+       finish_fault = (ulong)&finish_write_fault;
+
+       /*
+        * execute the vmwrite instruction that will cause a #PF
+        */
+       handler_called = false;
+       asm volatile ("movb %[byte], %%ah\n\t"
+                     "sahf\n\t"
+                     "vmwrite %[val], %[enc]; finish_write_fault:"
+                     : [val] "=m" (*(u64 *)vpage)
+                     : [byte] "Krm" (sentinel),
+                     [enc] "r" ((u64)GUEST_SEL_SS)
+                     : "cc", "ah");
+       report(handler_called, "The #PF handler was invoked");
+
+       /*
+        * restore the old #PF handler
+        */
+       handle_exception(PF_VECTOR, old);
+}
+
+static void test_vmwrite_flags_touch(void)
+{
+       /*
+        * set up the sentinel value in the flags register. we
+        * choose these two values because they candy-stripe
+        * the 5 flags that sahf sets.
+        */
+       sentinel = 0x91;
+       test_write_sentinel();
+
+       sentinel = 0x45;
+       test_write_sentinel();
+}
+
+
 static void test_vmcs_high(void)
 {
        struct vmcs *vmcs = alloc_page();

@@ -1988,6 +2124,10 @@ int main(int argc, const char *argv[])
                test_vmcs_lifecycle();
        if (test_wanted("test_vmx_caps", argv, argc))
                test_vmx_caps();
+       if (test_wanted("test_vmread_flags_touch", argv, argc))
+               test_vmread_flags_touch();
+       if (test_wanted("test_vmwrite_flags_touch", argv, argc))
+               test_vmwrite_flags_touch();
 
        /* Balance vmxon from test_vmxon. */
        vmx_off();
--

2.26.1.301.g55bc3eb7cb9-goog

Ping! (I realized I sent this via non-plaintext and the listserv rejected it.)

Quoted 190 lines

This commit adds new unit tests for commit a4d956b93904 ("KVM: nVMX:
vmread should not set rflags to specify success in case of #PF")

The two new tests force a vmread and a vmwrite on an unmapped address to cause a #PF and verify that the low byte of %rflags is preserved and that %rip is not advanced. The commit fixed a bug in vmread, but we include a test for vmwrite as well for completeness.

Before the aforementioned commit, the ALU flags would be incorrectly cleared and %rip would be advanced (for vmread).

Signed-off-by Simon Smith <[email protected]>

Reviewed-by Jim Mattson <[email protected]>

Reviewed-by Peter Shier <[email protected]>

Reviewed-by Krish Sadhukhan <[email protected]>

Reviewed-by Oliver Upton <[email protected]>

---
 x86/vmx.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

x86/vmx.c +140 -1

index 4c47eec1a1597..cbe68761894d4 100644
@@ -32,6 +32,7 @@
 #include "processor.h"
 #include "alloc_page.h"
 #include "vm.h"
+#include "vmalloc.h"
 #include "desc.h"
 #include "vmx.h"
 #include "msr.h"

@@ -387,6 +388,141 @@ static void test_vmwrite_vmread(void)
        free_page(vmcs);
 }
 
+ulong finish_fault;
+u8 sentinel;
+bool handler_called;
+
+static void pf_handler(struct ex_regs *regs)
+{
+       /*
+        * check that RIP was not improperly advanced and that the
+        * flags value was preserved.
+        */
+       report(regs->rip < finish_fault, "RIP has not been advanced!");
+       report(((u8)regs->rflags == ((sentinel | 2) & 0xd7)),
+              "The low byte of RFLAGS was preserved!");
+       regs->rip = finish_fault;
+       handler_called = true;
+
+}
+
+static void prep_flags_test_env(void **vpage, struct vmcs **vmcs, handler *old)
+{
+       /*
+        * get an unbacked address that will cause a #PF
+        */
+       *vpage = alloc_vpage();
+
+       /*
+        * set up VMCS so we have something to read from
+        */
+       *vmcs = alloc_page();
+
+       memset(*vmcs, 0, PAGE_SIZE);
+       (*vmcs)->hdr.revision_id = basic.revision;
+       assert(!vmcs_clear(*vmcs));
+       assert(!make_vmcs_current(*vmcs));
+
+       *old = handle_exception(PF_VECTOR, &pf_handler);
+}
+
+static void test_read_sentinel(void)
+{
+       void *vpage;
+       struct vmcs *vmcs;
+       handler old;
+
+       prep_flags_test_env(&vpage, &vmcs, &old);
+
+       /*
+        * set the proper label
+        */
+       extern char finish_read_fault;
+
+       finish_fault = (ulong)&finish_read_fault;
+
+       /*
+        * execute the vmread instruction that will cause a #PF
+        */
+       handler_called = false;
+       asm volatile ("movb %[byte], %%ah\n\t"
+                     "sahf\n\t"
+                     "vmread %[enc], %[val]; finish_read_fault:"
+                     : [val] "=m" (*(u64 *)vpage)
+                     : [byte] "Krm" (sentinel),
+                     [enc] "r" ((u64)GUEST_SEL_SS)
+                     : "cc", "ah");
+       report(handler_called, "The #PF handler was invoked");
+
+       /*
+        * restore the old #PF handler
+        */
+       handle_exception(PF_VECTOR, old);
+}
+
+static void test_vmread_flags_touch(void)
+{
+       /*
+        * set up the sentinel value in the flags register. we
+        * choose these two values because they candy-stripe
+        * the 5 flags that sahf sets.
+        */
+       sentinel = 0x91;
+       test_read_sentinel();
+
+       sentinel = 0x45;
+       test_read_sentinel();
+}
+
+static void test_write_sentinel(void)
+{
+       void *vpage;
+       struct vmcs *vmcs;
+       handler old;
+
+       prep_flags_test_env(&vpage, &vmcs, &old);
+
+       /*
+        * set the proper label
+        */
+       extern char finish_write_fault;
+
+       finish_fault = (ulong)&finish_write_fault;
+
+       /*
+        * execute the vmwrite instruction that will cause a #PF
+        */
+       handler_called = false;
+       asm volatile ("movb %[byte], %%ah\n\t"
+                     "sahf\n\t"
+                     "vmwrite %[val], %[enc]; finish_write_fault:"
+                     : [val] "=m" (*(u64 *)vpage)
+                     : [byte] "Krm" (sentinel),
+                     [enc] "r" ((u64)GUEST_SEL_SS)
+                     : "cc", "ah");
+       report(handler_called, "The #PF handler was invoked");
+
+       /*
+        * restore the old #PF handler
+        */
+       handle_exception(PF_VECTOR, old);
+}
+
+static void test_vmwrite_flags_touch(void)
+{
+       /*
+        * set up the sentinel value in the flags register. we
+        * choose these two values because they candy-stripe
+        * the 5 flags that sahf sets.
+        */
+       sentinel = 0x91;
+       test_write_sentinel();
+
+       sentinel = 0x45;
+       test_write_sentinel();
+}
+
+
 static void test_vmcs_high(void)
 {
        struct vmcs *vmcs = alloc_page();

@@ -1988,6 +2124,10 @@ int main(int argc, const char *argv[])
                test_vmcs_lifecycle();
        if (test_wanted("test_vmx_caps", argv, argc))
                test_vmx_caps();
+       if (test_wanted("test_vmread_flags_touch", argv, argc))
+               test_vmread_flags_touch();
+       if (test_wanted("test_vmwrite_flags_touch", argv, argc))
+               test_vmwrite_flags_touch();
 
        /* Balance vmxon from test_vmxon. */
        vmx_off();
--

2.26.1.301.g55bc3eb7cb9-goog