linux-kernel - Re: [PATCH v4 18/30] KVM: x86/mmu: Zap only TDP MMU leafs in kvm_zap_gfn

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]

Message-ID: <YiJdeaW7bLGmjSuf@google.com>
Date:   Fri, 4 Mar 2022 18:42:01 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Mingwei Zhang <mizhang@...gle.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, linux-kernel@...r.kernel.org,
        kvm@...r.kernel.org, Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        David Hildenbrand <david@...hat.com>,
        David Matlack <dmatlack@...gle.com>,
        Ben Gardon <bgardon@...gle.com>
Subject: Re: [PATCH v4 18/30] KVM: x86/mmu: Zap only TDP MMU leafs in
 kvm_zap_gfn_range()

On Fri, Mar 04, 2022, Mingwei Zhang wrote:
> On Fri, Mar 04, 2022, Sean Christopherson wrote:
> > On Fri, Mar 04, 2022, Mingwei Zhang wrote:
> > > On Thu, Mar 03, 2022, Paolo Bonzini wrote:
> > > > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > > > index f3939ce4a115..c71debdbc732 100644
> > > > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > > > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > > > @@ -834,10 +834,8 @@ bool kvm_tdp_mmu_zap_sp(struct kvm *kvm, struct kvm_mmu_page *sp)
> > > >  }
> > > >  
> > > >  /*
> > > > - * Tears down the mappings for the range of gfns, [start, end), and frees the
> > > > - * non-root pages mapping GFNs strictly within that range. Returns true if
> > > > - * SPTEs have been cleared and a TLB flush is needed before releasing the
> > > > - * MMU lock.
> > > > + * Zap leafs SPTEs for the range of gfns, [start, end). Returns true if SPTEs
> > > > + * have been cleared and a TLB flush is needed before releasing the MMU lock.
> > > 
> > > I think the original code does not _over_ zapping. But the new version
> > > does.
> > 
> > No, the new version doesn't overzap.
> 
> It does overzap, but it does not matter and the semantic does not
> change.

Belaboring the point a bit... it very much matters, KVM must "overzap" for functional
correctness.  It's only an "overzap" from the perspective that KVM could thoeretically
shatter the hugepage then zap only the relevant small pages.  But it's not an overzap
in the sense that KVM absolutely has to zap the hugepage.  Even if KVM replaces it
with a shadow page, the hugepage is still being zapped, i.e. it's gone and KVM must do
a TLB flush regardless of whether or not there's a new mapping.