linux-kernel - Re: [PATCH] KVM: x86/mmu: fix KVM_X86_QUIRK_SLOT_ZAP

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: <ZwP1kvgyIGIO_p0x@google.com>
Date: Mon, 7 Oct 2024 08:07:28 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [PATCH] KVM: x86/mmu: fix KVM_X86_QUIRK_SLOT_ZAP_ALL for shadow MMU

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.