| Message ID | 1469053536-11130-5-git-send-email-bsd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21/07/2016 00:25, Bandan Das wrote: > vmentry should check whether the vmcs provided by > the guest hypervisor is a shadow vmcs and fail. How can this happen, since vmptrld checks the revision_id as you said below? Paolo > Also, vmptrld should check whether a shadow vmcs > is being loaded by the guest without support being present > but this check happens as part of checking the revision_id. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: > On 21/07/2016 00:25, Bandan Das wrote: >> vmentry should check whether the vmcs provided by >> the guest hypervisor is a shadow vmcs and fail. > > How can this happen, since vmptrld checks the revision_id as you said below? This is more of a change that adheres to the spec (26.1 Basic VM-Entry Checks); the failure path is slightly different compared to vmptrld though. It's small and harmless but I am ok if you prefer dropping it. Thanks for the review! > Paolo > >> Also, vmptrld should check whether a shadow vmcs >> is being loaded by the guest without support being present >> but this check happens as part of checking the revision_id. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Paolo Bonzini <pbonzini@redhat.com> writes: > > > On 21/07/2016 00:25, Bandan Das wrote: > >> vmentry should check whether the vmcs provided by > >> the guest hypervisor is a shadow vmcs and fail. > > > > How can this happen, since vmptrld checks the revision_id as you said > > below? > > This is more of a change that adheres to the spec > (26.1 Basic VM-Entry Checks); the failure path > is slightly different compared to vmptrld though. > It's small and harmless but I am ok if you prefer dropping it. Do you mean that this could happen if the VMCS is modified by L1 after VMPTRLD? That makes sense, but with David Matlack's change to cache the VMCS it wouldn't be possible to trigger it anymore. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo Bonzini <pbonzini@redhat.com> writes: >> Paolo Bonzini <pbonzini@redhat.com> writes: >> >> > On 21/07/2016 00:25, Bandan Das wrote: >> >> vmentry should check whether the vmcs provided by >> >> the guest hypervisor is a shadow vmcs and fail. >> > >> > How can this happen, since vmptrld checks the revision_id as you said >> > below? >> >> This is more of a change that adheres to the spec >> (26.1 Basic VM-Entry Checks); the failure path >> is slightly different compared to vmptrld though. >> It's small and harmless but I am ok if you prefer dropping it. > > Do you mean that this could happen if the VMCS is modified by L1 > after VMPTRLD? That makes sense, but with David Matlack's change Yeah that's the only way I can see it happen. If there's a separate path that takes care of this, should I drop this one ? > to cache the VMCS it wouldn't be possible to trigger it anymore. > > Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 6291143..1b6f624 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -9924,6 +9924,10 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) skip_emulated_instruction(vcpu); vmcs12 = get_vmcs12(vcpu); + if ((vmcs12->revision_id >> 31) & 1u) { + nested_vmx_failInvalid(vcpu); + return 1; + } if (enable_shadow_vmcs) copy_shadow_to_vmcs12(vmx);
vmentry should check whether the vmcs provided by the guest hypervisor is a shadow vmcs and fail. Also, vmptrld should check whether a shadow vmcs is being loaded by the guest without support being present but this check happens as part of checking the revision_id. Signed-off-by: Bandan Das <bsd@redhat.com> --- arch/x86/kvm/vmx.c | 4 ++++ 1 file changed, 4 insertions(+)