Re: [PATCH 5.10.y 12/17] KVM: x86/mmu: Check PDPTRs before allocating PAE roots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Calling mmu->get_pdptr() (which can sleep) while holding the vcpu->kvm->mmu_lock spinlock, leading to sleep-in-atomic bugs.
--

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

KVM: x86/mmu: Check PDPTRs before allocating PAE roots

This commit adds a validity check for PDPTRs before allocating any of the PAE
roots. It iterates over the PDPTRs and checks if they are valid to prevent KVM
from leaking previously allocated roots when a bad PDPTR is encountered.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3308,7 +3308,7 @@ static int mmu_alloc_direct_roots(struct kvm_vcpu *vcpu)
>  static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_mmu *mmu = vcpu->arch.mmu;
> -	u64 pdptr, pm_mask;
> +	u64 pdptrs[4], pm_mask;
>  	gfn_t root_gfn, root_pgd;
>  	int quadrant, i;
>  	hpa_t root;
> @@ -3319,6 +3319,17 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
>  	if (mmu_check_root(vcpu, root_gfn))
>  		return 1;
>  
> +	if (mmu->root_level == PT32E_ROOT_LEVEL) {
> +		for (i = 0; i < 4; ++i) {
> +			pdptrs[i] = mmu->get_pdptr(vcpu, i);

[Severity: High]
Does this sequence call mmu->get_pdptr() while holding a spinlock?

Looking at kvm_mmu_load(), it calls mmu_alloc_shadow_roots() while holding
vcpu->kvm->mmu_lock:

kvm_mmu_load()
    spin_lock(&vcpu->kvm->mmu_lock);
    ...
    r = mmu_alloc_shadow_roots(vcpu);

If the vCPU is running a nested guest (L2) with a 32-bit PAE NPT, the callback
for mmu->get_pdptr() is nested_svm_get_tdp_pdptr(). This function reads guest
memory via kvm_vcpu_read_guest_page(), which can sleep.

Can this lead to a sleep-in-atomic issue when the lock is held?

> +			if (!(pdptrs[i] & PT_PRESENT_MASK))
> +				continue;
> +
> +			if (mmu_check_root(vcpu, pdptrs[i] >> PAGE_SHIFT))
> +				return 1;
> +		}
> +	}
> +
>  	/*
>  	 * Do we shadow a long mode page table? If so we need to
>  	 * write-protect the guests page table root.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626112634.1778506-1-pbonzini@redhat.com?part=12