Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1447513 > unrolled thread

[PATCH 4/4] nvmx: check for shadow vmcs check on entry

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/4] nvmx: check for shadow vmcs check on entry Bandan Das <bsd@redhat.com> - 2016-07-21 00:30 +0200
    Re: [PATCH 4/4] nvmx: check for shadow vmcs check on entry Paolo Bonzini <pbonzini@redhat.com> - 2016-07-21 11:20 +0200
      Re: [PATCH 4/4] nvmx: check for shadow vmcs check on entry Bandan Das <bsd@redhat.com> - 2016-07-21 21:30 +0200
        Re: [PATCH 4/4] nvmx: check for shadow vmcs check on entry Paolo Bonzini <pbonzini@redhat.com> - 2016-07-22 10:50 +0200
          Re: [PATCH 4/4] nvmx: check for shadow vmcs check on entry Bandan Das <bsd@redhat.com> - 2016-07-22 18:00 +0200

#1447513 — [PATCH 4/4] nvmx: check for shadow vmcs check on entry

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(+)

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);
-- 
2.5.5

[toc] | [next] | [standalone]

#1447725

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.

[toc] | [prev] | [next] | [standalone]

#1448093

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.

[toc] | [prev] | [next] | [standalone]

#1448474

> 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

[toc] | [prev] | [next] | [standalone]

#1448653

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web