From: Avi Kivity <avi@redhat.com>
To: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>,
Marcelo Tosatti <mtosatti@redhat.com>,
kvm@vger.kernel.org
Subject: [PATCH v4 7/8] KVM: MMU: Validate all gptes during fetch, not just those used for new pages
Date: Tue, 13 Jul 2010 14:27:10 +0300 [thread overview]
Message-ID: <1279020431-31431-8-git-send-email-avi@redhat.com> (raw)
In-Reply-To: <1279020431-31431-1-git-send-email-avi@redhat.com>
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 <avi@redhat.com>
---
arch/x86/kvm/paging_tmpl.h | 33 ++++++++++++++++++++++++---------
1 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index a550957..284ab16 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -319,10 +319,11 @@ 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 *sp = NULL;
u64 *sptep = NULL;
int uninitialized_var(level);
bool dirty = is_dirty_gpte(gw->ptes[gw->level - 1]);
+ int top_level;
unsigned direct_access;
struct kvm_shadow_walk_iterator iterator;
@@ -333,6 +334,18 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
if (!dirty)
direct_access &= ~ACC_WRITE_MASK;
+ top_level = vcpu->arch.mmu.root_level;
+ if (top_level == PT32E_ROOT_LEVEL)
+ top_level = PT32_ROOT_LEVEL;
+ /*
+ * Verify that the top-level gpte is still there. Since the page
+ * is a root page, it is either write protected (and cannot be
+ * changed from now on) or it is invalid (in which case, we don't
+ * really care if it changes underneath us after this point).
+ */
+ if (FNAME(gpte_changed)(vcpu, gw, top_level))
+ goto out_gpte_changed;
+
for (shadow_walk_init(&iterator, vcpu, addr);
shadow_walk_okay(&iterator) && iterator.level > gw->level;
shadow_walk_next(&iterator)) {
@@ -343,12 +356,12 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
drop_large_spte(vcpu, sptep);
- if (is_shadow_present_pte(*sptep))
- continue;
-
- table_gfn = gw->table_gfn[level - 2];
- sp = kvm_mmu_get_page(vcpu, table_gfn, addr, level-1,
- false, access, sptep);
+ sp = NULL;
+ 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);
+ }
/*
* Verify that the gpte in the page we've just write
@@ -357,7 +370,8 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
if (FNAME(gpte_changed)(vcpu, gw, level - 1))
goto out_gpte_changed;
- link_shadow_page(sptep, sp);
+ if (sp)
+ link_shadow_page(sptep, sp);
}
for (;
@@ -392,7 +406,8 @@ static u64 *FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr,
return sptep;
out_gpte_changed:
- kvm_mmu_put_page(sp, sptep);
+ if (sp)
+ kvm_mmu_put_page(sp, sptep);
kvm_release_pfn_clean(pfn);
return NULL;
}
--
1.7.1
next prev parent reply other threads:[~2010-07-13 11:27 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-13 11:27 [PATCH v4 0/8] Simplify and fix fetch() Avi Kivity
2010-07-13 11:27 ` [PATCH v4 1/8] KVM: MMU: Add link_shadow_page() helper Avi Kivity
2010-07-13 11:27 ` [PATCH v4 2/8] KVM: MMU: Use __set_spte to link shadow pages Avi Kivity
2010-07-13 11:27 ` [PATCH v4 3/8] KVM: MMU: Add drop_large_spte() helper Avi Kivity
2010-07-13 11:27 ` [PATCH v4 4/8] KVM: MMU: Add validate_direct_spte() helper Avi Kivity
2010-07-13 11:27 ` [PATCH v4 5/8] KVM: MMU: Add gpte_valid() helper Avi Kivity
2010-07-13 11:27 ` [PATCH v4 6/8] KVM: MMU: Simplify spte fetch() function Avi Kivity
2010-07-13 11:27 ` Avi Kivity [this message]
2010-07-13 11:27 ` [PATCH v4 8/8] KVM: MMU: Eliminate redundant temporaries in FNAME(fetch) Avi Kivity
2010-07-13 11:50 ` [PATCH v4 0/8] Simplify and fix fetch() Avi Kivity
2010-07-14 0:09 ` Marcelo Tosatti
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1279020431-31431-8-git-send-email-avi@redhat.com \
--to=avi@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=mtosatti@redhat.com \
--cc=xiaoguangrong@cn.fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox