Re: [PATCH v11 08/11] KVM: TDX: Get/put PAMT pages when (un)mapping private memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The retry loop in `kvm_tdp_mmu_map_private_pfn` fails to top up memory caches on `RET_PF_RETRY`, leading to a permanent infinite loop when `pamt_cache` is depleted.
--

commit 346b744d7586de76e5cf17e0acaaeca78c9eea73
Author: Kiryl Shutsemau <kas@kernel.org>

KVM: TDX: Get/put PAMT pages when (un)mapping private memory

This commit adds Dynamic PAMT support to KVM's S-EPT MMU by getting PAMT
pages when adding guest memory and putting them when removing guest memory.
It uses per-vCPU PAMT caches topped up via a new topup_external_cache op.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 22cf222d30339..48e4cf91b77f6 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -614,6 +614,10 @@ static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu, bool maybe_indirect)
>  					       PT64_ROOT_MAX_LEVEL);
>  		if (r)
>  			return r;
> +
> +		r = kvm_x86_call(topup_external_cache)(vcpu, PT64_ROOT_MAX_LEVEL);
> +		if (r)
> +			return r;
>  	}

[Severity: High]
Does this code guarantee eventual cache exhaustion during page faults?

When a host userspace process initiates KVM_TDX_INIT_MEM_REGION, it calls
kvm_tdp_mmu_map_private_pfn() in arch/x86/kvm/mmu/mmu.c. In that function,
mmu_topup_memory_caches() is located outside the retry loop:

	r = mmu_topup_memory_caches(vcpu, false);
	if (r)
		return r;

	do {
		if (signal_pending(current))
			return -EINTR;

		if (kvm_test_request(KVM_REQ_VM_DEAD, vcpu))
			return -EIO;

		cond_resched();
		guard(read_lock)(&kvm->mmu_lock);

		r = kvm_tdp_mmu_map(vcpu, &fault);
	} while (r == RET_PF_RETRY);

During kvm_tdp_mmu_map(), if tdh_mem_page_add() or tdh_mem_sept_add()
encounters TDX_OPERAND_BUSY due to contention, tdx_pamt_put() frees the
just-allocated PAMT page to the generic kernel buddy allocator instead of
returning it to the cache.

kvm_tdp_mmu_map() then receives -EBUSY from tdp_mmu_set_spte_atomic() and
translates it to RET_PF_RETRY. Because the topup is outside the loop, the
pamt_cache is never replenished upon retry.

[ ... ]
> diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> index 957078e8656ef..09573112540e9 100644
> --- a/arch/x86/kvm/vmx/tdx.c
> +++ b/arch/x86/kvm/vmx/tdx.c
[ ... ]
> @@ -1701,8 +1728,13 @@ static int tdx_sept_map_nonleaf_spte(struct kvm *kvm, gfn_t gfn,
>  static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level level,
>  				  u64 new_spte)
>  {
> +	struct kvm_vcpu *vcpu = kvm_get_running_vcpu();
>  	struct kvm_tdx *kvm_tdx = to_kvm_tdx(kvm);
>  	kvm_pfn_t pfn = spte_to_pfn(new_spte);
> +	int ret;
> +
> +	if (KVM_BUG_ON(!vcpu, kvm))
> +		return -EIO;
>  
>  	/* TODO: handle large pages. */
>  	if (KVM_BUG_ON(level != PG_LEVEL_4K, kvm))
> @@ -1710,6 +1742,10 @@ static int tdx_sept_map_leaf_spte(struct kvm *kvm, gfn_t gfn, enum pg_level leve
>  
>  	WARN_ON_ONCE((new_spte & VMX_EPT_RWX_MASK) != VMX_EPT_RWX_MASK);
>  
> +	ret = tdx_pamt_get(pfn, &to_tdx(vcpu)->pamt_cache);
> +	if (KVM_BUG_ON(ret, kvm))
> +		return ret;

[Severity: High]
Does this failure path cause a permanent infinite loop?

After the cache is completely drained due to the missing topups on retry,
alloc_dpamt_page() in arch/x86/virt/vmx/tdx/tdx.c returns NULL because it
lacks a GFP_ATOMIC fallback for a non-NULL cache:

static struct page *alloc_dpamt_page(struct tdx_pamt_cache *cache)
{
	if (cache)
		return tdx_alloc_page_pamt_cache(cache);

	return alloc_page(GFP_KERNEL_ACCOUNT);
}

Consequently, tdx_pamt_get() fails with -ENOMEM. This is subsequently treated
as a retryable error by tdp_mmu_map_handle_target_level() which returns
RET_PF_RETRY.

This causes the host kernel thread executing the page population ioctl to
blindly repeat forever, creating a viable denial of service vector for
unprivileged host userspace.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904215841.303070-1-rick.p.edgecombe@intel.com?part=8