Re: [PATCH] KVM: x86/mmu: fix KVM_X86_QUIRK_SLOT_ZAP_ALL for shadow MMU

[Sean Christopherson]

On Fri, Oct 04, 2024, Sean Christopherson wrote:
> On Thu, Oct 03, 2024, Paolo Bonzini wrote:
> > +static void kvm_mmu_zap_memslot(struct kvm *kvm,
> > +				struct kvm_memory_slot *slot)
> >  {
> >  	struct kvm_gfn_range range = {
> >  		.slot = slot,
> > @@ -7064,11 +7096,11 @@ static void kvm_mmu_zap_memslot_leafs(struct kvm *kvm, struct kvm_memory_slot *s
> >  		.end = slot->base_gfn + slot->npages,
> >  		.may_block = true,
> >  	};
> > +	bool flush;
> >  
> >  	write_lock(&kvm->mmu_lock);
> > -	if (kvm_unmap_gfn_range(kvm, &range))
> > -		kvm_flush_remote_tlbs_memslot(kvm, slot);
> > -
> > +	flush = kvm_unmap_gfn_range(kvm, &range);
> 
> Aha!  Finally figured out why this was bugging me.  Using kvm_unmap_gfn_range()
> is subject to a race that would lead to UAF.  Huh.  And that could explain the
> old VFIO bug, though it seems unlikely that the race was being hit.
> 
>   KVM_SET_USER_MEMORY_REGION             vCPU
>                                          __kvm_faultin_pfn() /* resolve fault->pfn */
>   kvm_swap_active_memslots();
>   kvm_zap_gfn_range(APIC);

Copy+paste fail, this was supposed to be synchronize_srcu_expedited().

>   kvm_mmu_zap_memslot();
>                                         {read,write}_lock(&kvm->mmu_lock);
>                                         <install SPTE>
> 
> KVM's existing memslot deletion relies on the mmu_valid_gen check in is_obsolete_sp()
> to detect an obsolete root (and the KVM_REQ_MMU_FREE_OBSOLETE_ROOTS check to handle
> roots without a SP).
> 
> With this approach, roots aren't invalidated, and so a vCPU could install a SPTE
> using the to-be-delete memslot.

This is wrong, I managed to forget kvm->srcu is held for the entire duration of
KVM_RUN (except for the actual VM-Enter/VM-Exit code).  And the slot is retrieved
before the mmu_invalidate_seq snapshot is taken.