From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 7/7] KVM: MMU: Validate all gptes during fetch, not just those used for new pages Date: Mon, 12 Jul 2010 11:58:55 +0300 Message-ID: <4C3AD94F.8030809@redhat.com> References: <1278862955-6890-1-git-send-email-avi@redhat.com> <1278862955-6890-8-git-send-email-avi@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit To: Xiao Guangrong , Marcelo Tosatti , kvm@vger.kernel.org Return-path: Received: from mx1.redhat.com ([209.132.183.28]:49756 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752871Ab0GLI66 (ORCPT ); Mon, 12 Jul 2010 04:58:58 -0400 In-Reply-To: <1278862955-6890-8-git-send-email-avi@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 07/11/2010 06:42 PM, Avi Kivity wrote: > Currently, when we fetch an spte, we only verify that gptes match those that > the walker saw if we build new shadow pages for them. > > However, this misses the following race: > > vcpu1 vcpu2 > > walk > change gpte > walk > instantiate sp > > fetch existing sp > > Fix by validating every gpte, regardless of whether it is used for building > a new sp or not. > > Signed-off-by: Avi Kivity > --- > arch/x86/kvm/paging_tmpl.h | 18 ++++++++++-------- > 1 files changed, 10 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index a7f8295..4bbd0c7 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -325,7 +325,7 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr, > int *ptwrite, pfn_t pfn) > { > unsigned access = gw->pt_access; > - struct kvm_mmu_page *sp; > + struct kvm_mmu_page *uninitialized_var(sp); > u64 *sptep = NULL; > int uninitialized_var(level); > bool dirty = is_dirty_gpte(gw->ptes[gw->level - 1]); > @@ -343,18 +343,19 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr, > shadow_walk_okay(&iterator)&& iterator.level> gw->level; > shadow_walk_next(&iterator)) { > gfn_t table_gfn; > + bool new_page = false; > > level = iterator.level; > sptep = iterator.sptep; > > drop_spte_if_large(vcpu, sptep); > > - if (is_shadow_present_pte(*sptep)) > - continue; > See, this gets dropped. > - > - table_gfn = gw->table_gfn[level - 2]; > - sp = kvm_mmu_get_page(vcpu, table_gfn, addr, level-1, > - false, access, sptep); > + if (!is_shadow_present_pte(*sptep)) { > + table_gfn = gw->table_gfn[level - 2]; > + sp = kvm_mmu_get_page(vcpu, table_gfn, addr, level-1, > + false, access, sptep); > + new_page = true; > + } > > if (!FNAME(validate_indirect_spte)(vcpu, sptep, sp, > gw, level)) { > Now we need to change validate_indirect_spte() to account for all levels. -- error compiling committee.c: too many arguments to function