Re: [PATCH v6 05/13] KVM: nVMX: implement enlightened VMPTRLD and VMCLEAR

[Vitaly Kuznetsov]

Jim Mattson <jmattson@xxxxxxxxxx> writes:

> On Tue, Oct 16, 2018 at 9:50 AM Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> wrote:
>>
>> +
>> +               vmx->nested.hv_evmcs = kmap(vmx->nested.hv_evmcs_page);
>
> Are you sure that directly mapping guest memory isn't going to lead to
> time-of-check vs. time-of-use bugs? This is a very hard programming
> model to get right.

The basic assumption here is that Enlightened VMCS (just like normal or
shadow VMCSes) is being access by one L1 vCPU only. When we access it
from KVM the vCPU is not running. Yes, L1 guest can screw itself up by
breaking this assumption but honestly I don't see how this is different
from normal VMCS: we can always break things by writing to the page from
a different vCPU.

Enlightened VMCS is (mostly) not used directly: we copy it to vmcs12 and
then back before entry. The only field we always access directly is
hv_clean_fields. We can, of course, copy it to vmcs12 too but I failed
to find a reason to do so: L1 guest is in control of the field, it can
always write junk there and L2 guest will likely get broken.

I remember having map/copy/unmap sequences for eVMCS on entry/exit in
some early version of this series but it was just slowing things down so
I switched to having it permanently mapped. In case you see (potential)
grave bugs with this we can of course re-consider.

Thanks,

-- 
Vitaly