linux-kernel - Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 18 Aug 2017 14:35:50 +0200
From:   Radim Krčmář <rkrcmar@...hat.com>
To:     David Hildenbrand <david@...hat.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, wanpeng.li@...mail.com
Subject: Re: [PATCH 3/3] KVM: x86: fix use of L1 MMIO areas in nested guests

2017-08-18 09:59+0200, David Hildenbrand:
> On 17.08.2017 18:36, Paolo Bonzini wrote:
> > There is currently some confusion between nested and L1 GPAs.  The
> > assignment to "direct" in kvm_mmu_page_fault tries to fix that, but
> > it is not enough.  What this patch does is fence off the MMIO cache
> > completely when using shadow nested page tables, since we have neither
> > a GVA nor an L1 GPA to put in the cache.  This also allows some
> > simplifications in kvm_mmu_page_fault and FNAME(page_fault).
> > 
> > The EPT misconfig likewise does not have an L1 GPA to pass to
> > kvm_io_bus_write, so that must be skipped for guest mode.
> > 
> > Signed-off-by: Paolo Bonzini <pbonzini@...hat.com>
> > ---
> > 	v1->v2: standardize on "nGPA" moniker, replace nested ifs with &&
> > 
> >  arch/x86/kvm/mmu.c         | 10 +++++++++-
> >  arch/x86/kvm/paging_tmpl.h |  3 +--
> >  arch/x86/kvm/vmx.c         |  7 ++++++-
> >  arch/x86/kvm/x86.h         |  6 +++++-
> >  4 files changed, 21 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> > index a2c592b14617..02f8c507b160 100644
> > --- a/arch/x86/kvm/mmu.c
> > +++ b/arch/x86/kvm/mmu.c
> > @@ -3596,6 +3596,14 @@ static bool is_shadow_zero_bits_set(struct kvm_mmu *mmu, u64 spte, int level)
> >  
> >  static bool mmio_info_in_cache(struct kvm_vcpu *vcpu, u64 addr, bool direct)
> >  {
> > +	/*
> > +	 * A nested guest cannot use the MMIO cache if it is using nested
> > +	 * page tables, because cr2 is a nGPA while the cache stores L1's
> > +	 * physical addresses.
> 
> ... "while the cache stores GPAs" ?

Makes sense, changed while applying.

> > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
> > @@ -90,7 +90,11 @@ static inline u32 bit(int bitno)
> >  static inline void vcpu_cache_mmio_info(struct kvm_vcpu *vcpu,
> >  					gva_t gva, gfn_t gfn, unsigned access)
> >  {
> > -	vcpu->arch.mmio_gva = gva & PAGE_MASK;
> > +	/*
> > +	 * If this is a shadow nested page table, the "GVA" is
> 
> s/"GVA"/GVA/ ?

I prefer the former, we're talking about "gva_t gva" that isn't GVA. :)