Re: [PATCH 1/4] KVM: nVMX: Check GUEST_SYSENTER_ESP and GUEST_SYSENTER_EIP on vmentry of nested guests

[Krish Sadhukhan <krish.sadhukhan@oracle.com>]

On 12/10/19 9:57 AM, Jim Mattson wrote:
> On Fri, Dec 6, 2019 at 3:49 PM Krish Sadhukhan
> <krish.sadhukhan@oracle.com> wrote:
>> According to section "Checks on Guest Control Registers, Debug Registers, and
>> and MSRs" in Intel SDM vol 3C, the following checks are performed on vmentry
>> of nested guests:
>>
>>      "The IA32_SYSENTER_ESP field and the IA32_SYSENTER_EIP field must each
>>       contain a canonical address."
>>
>> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
>> Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com>
>> ---
>>   arch/x86/kvm/vmx/nested.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>> index 0e7c9301fe86..a2d1c305a7d8 100644
>> --- a/arch/x86/kvm/vmx/nested.c
>> +++ b/arch/x86/kvm/vmx/nested.c
>> @@ -2770,6 +2770,10 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
>>              CC(!nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)))
>>                  return -EINVAL;
>>
>> +       if (CC(!is_noncanonical_address(vmcs12->guest_sysenter_esp)) ||
>> +           CC(!is_noncanonical_address(vmcs12->guest_sysenter_eip)))
>> +               return -EINVAL;
>> +
> Don't the hardware checks on the corresponding vmcs02 fields suffice
> in this case?

In prepare_vmcs02(), we have the following code:

         if (vmx->nested.dirty_vmcs12 || hv_evmcs) {
                 prepare_vmcs02_rare(vmx, vmcs12);

If vmcs12 is dirty, we are setting these two fields from vmcs12 and I 
thought the values needed to be checked in software. Did I miss something ?

>
>>          if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) &&
>>              CC(!kvm_pat_valid(vmcs12->guest_ia32_pat)))
>>                  return -EINVAL;
>> --
>> 2.20.1
>>