On 04/05/20 18:19, Simon Smith wrote:
> Ping! (I realized I sent this via non-plaintext and the listserv rejected it.)
Queued now, thanks!
Paolo
>
> On Mon, Apr 20, 2020 at 10:59 AM Simon Smith <brigidsmith@xxxxxxxxxx> wrote:
>>
>> 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 <brigidsmith@xxxxxxxxxx>
>> Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
>> Reviewed-by: Peter Shier <pshier@xxxxxxxxxx>
>> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
>> Reviewed-by: Oliver Upton <oupton@xxxxxxxxxx>
>> ---
>> x86/vmx.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 140 insertions(+)
>>
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index 4c47eec1a1597..cbe68761894d4 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -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
>>
>
