Re: [PATCH v2 6/9] KVM: x86/mmu: Split kvm_mmu_zap_all_fast() into "front" and "back" halves

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Evaluating `kvm_memslot_flush_zap_all(kvm)` twice in `kvm_arch_flush_shadow_memslot()` creates a TOCTOU race window that allows concurrent quirk modification to skip the back-half zap, leading to a Use-After-Free of host memory.
--

commit daadba9269f9868eb59f2fe4d2fd49715b55bc67
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Split kvm_mmu_zap_all_fast() into "front" and "back" halves

This commit splits kvm_mmu_zap_all_fast() into a front half (running with
mmu_lock held for write) and a back half (running outside of mmu_lock).
This allows sharing the mmu_lock critical section when zapping in response
to a memslot being deleted or moved.

Cc: stable@vger.kernel.org # 6.12.x

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 223d80b12b9b6..5925db37543f3 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c

[ ... ]

> @@ -7578,14 +7591,19 @@ void kvm_arch_flush_shadow_memslot(struct kvm *kvm,
>  	};
>  	bool flush;
>  
> +	write_lock(&kvm->mmu_lock);
> +
>  	if (kvm_memslot_flush_zap_all(kvm)) {
> -		kvm_mmu_zap_all_fast(kvm);
> +		__kvm_mmu_zap_all_fast_front_half(kvm);
>  	} else {
> -		write_lock(&kvm->mmu_lock);
>  		flush = kvm_unmap_gfn_range(kvm, &range);
>  		kvm_mmu_zap_memslot_pages_and_flush(kvm, slot, flush);
> -		write_unlock(&kvm->mmu_lock);
>  	}
> +
> +	write_unlock(&kvm->mmu_lock);
> +
> +	if (kvm_memslot_flush_zap_all(kvm))
> +		__kvm_mmu_zap_all_fast_back_half(kvm);
>  }

[Severity: Critical]
Can kvm_memslot_flush_zap_all() evaluate differently on the second check in
kvm_arch_flush_shadow_memslot()?

Since kvm_memslot_flush_zap_all() dynamically checks disabled_quirks, if a
concurrent thread disables KVM_X86_QUIRK_SLOT_ZAP_ALL after the front half
executes but before the back half check:

Thread 1 in kvm_arch_flush_shadow_memslot():
    if (kvm_memslot_flush_zap_all(kvm))
        __kvm_mmu_zap_all_fast_front_half(kvm);
        
Thread 2:
    KVM_ENABLE_CAP with KVM_CAP_DISABLE_QUIRKS
    (disables KVM_X86_QUIRK_SLOT_ZAP_ALL)
    
Thread 1 resumes:
    if (kvm_memslot_flush_zap_all(kvm))
        // Evaluates to false, skips back half
        __kvm_mmu_zap_all_fast_back_half(kvm);
        
Could this skip the back half zap and leave invalidated TDP MMU roots
unzapped? If so, this could lead to a use-after-free because KVM's MMU
notifiers will not find the mapping to zap it when the host frees the memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626231416.3943216-1-seanjc@google.com?part=6