Re: [PATCH 5.10] KVM: x86/mmu: Fix write-protection of PTs mapped by the TDP MMU

[Greg KH <gregkh@linuxfoundation.org>]

On Mon, Jan 24, 2022 at 07:34:13PM +0100, Paolo Bonzini wrote:
> On 1/24/22 19:33, David Matlack wrote:
> > commit 7c8a4742c4abe205ec9daf416c9d42fd6b406e8e upstream.
> > 
> > When the TDP MMU is write-protection GFNs for page table protection (as
> > opposed to for dirty logging, or due to the HVA not being writable), it
> > checks if the SPTE is already write-protected and if so skips modifying
> > the SPTE and the TLB flush.
> > 
> > This behavior is incorrect because it fails to check if the SPTE
> > is write-protected for page table protection, i.e. fails to check
> > that MMU-writable is '0'.  If the SPTE was write-protected for dirty
> > logging but not page table protection, the SPTE could locklessly be made
> > writable, and vCPUs could still be running with writable mappings cached
> > in their TLB.
> > 
> > Fix this by only skipping setting the SPTE if the SPTE is already
> > write-protected *and* MMU-writable is already clear.  Technically,
> > checking only MMU-writable would suffice; a SPTE cannot be writable
> > without MMU-writable being set.  But check both to be paranoid and
> > because it arguably yields more readable code.
> > 
> > Fixes: 46044f72c382 ("kvm: x86/mmu: Support write protection for nesting in tdp MMU")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: David Matlack <dmatlack@google.com>
> > Message-Id: <20220113233020.3986005-2-dmatlack@google.com>
> > Reviewed-by: Sean Christopherson <seanjc@google.com>
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > ---
> >   arch/x86/kvm/mmu/tdp_mmu.c | 6 +++---
> >   1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> > index f2ddf663e72e..7e08efb06839 100644
> > --- a/arch/x86/kvm/mmu/tdp_mmu.c
> > +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> > @@ -1130,12 +1130,12 @@ static bool write_protect_gfn(struct kvm *kvm, struct kvm_mmu_page *root,
> >   	bool spte_set = false;
> >   	tdp_root_for_each_leaf_pte(iter, root, gfn, gfn + 1) {
> > -		if (!is_writable_pte(iter.old_spte))
> > -			break;
> > -
> >   		new_spte = iter.old_spte &
> >   			~(PT_WRITABLE_MASK | SPTE_MMU_WRITEABLE);
> > +		if (new_spte == iter.old_spte)
> > +			break;
> > +
> >   		tdp_mmu_set_spte(kvm, &iter, new_spte);
> >   		spte_set = true;
> >   	}
> > 
> > base-commit: fd187a4925578f8743d4f266c821c7544d3cddae
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>
> 

Now queued up, thanks.

greg k-h