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.
