From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [PATCH 05/10] KVM: nVMX: Allocate shadow vmcs Date: Thu, 18 Apr 2013 10:11:22 +0300 Message-ID: <20130418071122.GK8997@redhat.com> References: <1366218306-abelg@il.ibm.com> <20130417170740.3C8D87B802A@moren.haifa.ibm.com> <20130418063843.GG8997@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=cp1255 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: dongxiao.xu@intel.com, jun.nakajima@intel.com, kvm@vger.kernel.org, nadav@harel.org.il, owasserm@redhat.com To: Abel Gordon Return-path: Received: from mx1.redhat.com ([209.132.183.28]:30878 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752816Ab3DRHLa convert rfc822-to-8bit (ORCPT ); Thu, 18 Apr 2013 03:11:30 -0400 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Apr 18, 2013 at 10:07:11AM +0300, Abel Gordon wrote: >=20 >=20 > Gleb Natapov wrote on 18/04/2013 09:38:43 AM: >=20 > > On Wed, Apr 17, 2013 at 08:07:40PM +0300, Abel Gordon wrote: > > > Allocate a shadow vmcs used by the processor to shadow part of th= e > fields > > > stored in the software defined VMCS12 (let L1 access fields witho= ut > causing > > > exits). Note we keep a shadow vmcs only for the current vmcs12. > > Once a vmcs12 > > > becomes non-current, its shadow vmcs is released. > > > > > > > > > Signed-off-by: Abel Gordon > > > --- > > > arch/x86/kvm/vmx.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > --- .before/arch/x86/kvm/vmx.c 2013-04-17 19:58:32.000000000 +0= 300 > > > +++ .after/arch/x86/kvm/vmx.c 2013-04-17 19:58:32.000000000 +03= 00 > > > @@ -355,6 +355,7 @@ struct nested_vmx { > > > /* The host-usable pointer to the above */ > > > struct page *current_vmcs12_page; > > > struct vmcs12 *current_vmcs12; > > > + struct vmcs *current_shadow_vmcs; > > > > > > /* vmcs02_list cache of VMCSs recently used to run L2 guests = */ > > > struct list_head vmcs02_pool; > > > @@ -5517,6 +5518,7 @@ static int handle_vmon(struct kvm_vcpu * > > > { > > > struct kvm_segment cs; > > > struct vcpu_vmx *vmx =3D to_vmx(vcpu); > > > + struct vmcs *shadow_vmcs; > > > > > > /* The Intel VMX Instruction Reference lists a bunch of bits = that > > > * are prerequisite to running VMXON, most notably cr4.VMXE m= ust be > > > @@ -5540,6 +5542,16 @@ static int handle_vmon(struct kvm_vcpu * > > > kvm_inject_gp(vcpu, 0); > > > return 1; > > > } > > > + if (enable_shadow_vmcs) { > > > + shadow_vmcs =3D alloc_vmcs(); > > > + if (!shadow_vmcs) > > > + return -ENOMEM; > > > + /* mark vmcs as shadow */ > > > + shadow_vmcs->revision_id |=3D (1u << 31); > > > + /* init shadow vmcs */ > > > + vmcs_clear(shadow_vmcs); > > > + vmx->nested.current_shadow_vmcs =3D shadow_vmcs; > > > + } > > > > > Guest can ddos host by calling vmxon repeatedly causing host to lea= k > > memory. This point to a bug in vmxon implementation. vmxon should c= all > > nested_vmx_failInvalid() if (vmx->nested.vmxon). >=20 > Good point. I just checked the spec (VMXON pseudo-code) to verify > the right emulation: > According to the pseudo-code we should: > ELSE VMfail(=93VMXON executed in VMX root operation=94) which means= : >=20 > VMfail(ErrorNumber): > IF VMCS pointer is valid > THEN VMfailValid(ErrorNumber); > ELSE VMfailInvalid; > FI; >=20 >=20 > So, I'll call nested_vmx_failValid if nested.current_vmptr !=3D -1ull > Otherwise, I'll call nested_vmx_failInvalid. >=20 Just call nested_vmx_failValid(). It does that internally. -- Gleb.