Re: [PATCH] KVM: x86/mmu: WARN and flush TLBs if TDP MMU replaces present SPTE with MMIO SPTE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVM: x86/mmu: tdp_mmu_map_handle_target_level() flushes untruncated GFN on MMIO overwrite
--

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

KVM: x86/mmu: WARN and flush TLBs if TDP MMU replaces present SPTE with MMIO SPTE

This commit adds a warning and remote tlb flush in
tdp_mmu_map_handle_target_level() if a shadow-present spte is replaced with a
non-shadow-present mmio spte due to gfn truncation or wrapping during the
page table walk.

Link: https://lore.kernel.org/all/20260219002241.2908563-1-seanjc@google.com

> diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
> index 44dad106fad19..2735cbb212937 100644
> --- a/arch/x86/kvm/mmu/tdp_mmu.c
> +++ b/arch/x86/kvm/mmu/tdp_mmu.c
> @@ -1151,6 +1151,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
>  		return RET_PF_RETRY;
>  	else if (is_shadow_present_pte(iter->old_spte) &&
>  		 (!is_last_spte(iter->old_spte, iter->level) ||
> +		  WARN_ON_ONCE(!is_shadow_present_pte(new_spte)) ||
>  		  WARN_ON_ONCE(leaf_spte_change_needs_tlb_flush(iter->old_spte, new_spte))))
>  		kvm_flush_remote_tlbs_gfn(vcpu->kvm, iter->gfn, iter->level);

[Severity: High]
When a shadow-present spte mapping a valid memslot gfn is overwritten with a
non-shadow-present mmio spte due to gfn truncation in the tdp iterator, does
flushing iter->gfn via kvm_flush_remote_tlbs_gfn() flush the wrong address
when range-based tlb invalidation is used?

Suppose a guest with a 4-level root first faults in a valid memslot at a
lower gfn (caching that translation in the tlb), and then triggers a fault on
an aliased gfn with upper bits set above the root level where fault->slot is
null.

During kvm_tdp_mmu_map(), tdp_iter_restart() and try_step_down() compute
iter->gfn via gfn_round_for_level():

arch/x86/kvm/mmu/mmu_internal.h:gfn_round_for_level() {
	return gfn & -KVM_PAGES_PER_HPAGE(level);
}

which only masks low bits below iter->level and retains the untruncated upper
bits in iter->gfn. Meanwhile, tdp_iter_refresh_sptep() indexes the page table
by shifting and masking out the upper bits:

arch/x86/kvm/mmu/tdp_iter.c:tdp_iter_refresh_sptep() {
	iter->sptep = iter->pt_path[iter->level - 1] +
		SPTE_INDEX((iter->gfn | iter->gfn_bits) << PAGE_SHIFT, iter->level);
}

resolving iter->sptep to the existing spte for the lower gfn.

When tdp_mmu_map_handle_target_level() overwrites iter->old_spte with the
mmio spte and calls kvm_flush_remote_tlbs_gfn(vcpu->kvm, iter->gfn,
iter->level), it passes the untruncated faulting iter->gfn. In
kvm_flush_remote_tlbs_range():

virt/kvm/kvm_main.c:kvm_flush_remote_tlbs_range() {
	if (!kvm_arch_flush_remote_tlbs_range(kvm, gfn, nr_pages))
		return;
	...
	kvm_flush_remote_tlbs(kvm);
}

if range-based flushing is supported and succeeds (for example, via
hv_flush_remote_tlbs_range()), only the untruncated iter->gfn is invalidated
and kvm_flush_remote_tlbs() is skipped.

Because the spte is now a non-shadow-present mmio spte, subsequent mmu
notifier invalidations in tdp_mmu_zap_leafs() will skip it without flushing
tlbs, leaving the cached tlb entry for the original memslot gfn pointing to a
host pfn that can be freed.

Should this path call kvm_flush_remote_tlbs(vcpu->kvm) (or mask iter->gfn to
the root's address space) when !is_shadow_present_pte(new_spte) triggers?

>  
taker

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