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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 12/10/19 11:48 AM, Jim Mattson wrote:
On Tue, Dec 10, 2019 at 11:36 AM Krish Sadhukhan
<krish.sadhukhan@xxxxxxxxxx> wrote:

On 12/10/19 9:57 AM, Jim Mattson wrote:
On Fri, Dec 6, 2019 at 3:49 PM Krish Sadhukhan
<krish.sadhukhan@xxxxxxxxxx> 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@xxxxxxxxxx>
Reviewed-by: Karl Heubaum <karl.heubaum@xxxxxxxxxx>
---
   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 ?
Typically, "guest state" doesn't have to be checked in software, as
long as (a) the vmcs12 field is copied unmodified to the corresponding
vmcs02 field, and (b) the virtual CPU enforces the same constraints as
the physical CPU. In this case, if there is a problem with the guest
state, the VM-entry to vmcs02 will immediately VM-exit with "VM-entry
failure due to invalid guest state," and L0 will reflect this exit
reason to L1.


Thanks for the explanation !

So the kvm-unit-test is still needed to verify that hardware does the check. Right ?


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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux