Re: [PATCH v5 3/7] KVM: arm64: Top up stage-2 memcache for dirty logging faults

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A malicious nested (L1) guest can crash the KVM host via a Break-Before-Make (BBM) violation that triggers a BUG_ON() due to an empty memory cache during a permission fault.
--

--- Patch [3]: [PATCH v5 3/7] KVM: arm64: Top up stage-2 memcache for dirty logging faults ---
commit bac5488834df4c0a16b08356c8263e8eb67e0fe4
Author: Bradley Morgan <include@grrlz.net>

KVM: arm64: Top up stage-2 memcache for dirty logging faults

Dirty logging forces new stage-2 mappings to page size but does not always
split an existing block first. A non-write permission fault on such a block
still needs a page-table page to split it, but the top-up was previously
gated on write faults. This patch tops up the memcache for any permission
fault while dirty logging is active.

Link: https://lore.kernel.org/all/20260623165634.699011F000E9@smtp.kernel.org/

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 4d7c9bdcef004..74e7e7f7564c8 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -2113,14 +2113,13 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
>  	 * Permission faults just need to update the existing leaf entry,
>  	 * and so normally don't require allocations from the memcache. The
>  	 * only exception to this is when dirty logging is enabled at runtime
> -	 * and a write fault needs to collapse a block entry into a table.
> +	 * and a fault needs to collapse a block entry into a table.
>  	 * Under pKVM a permission fault can also collapse pages into a block,
>  	 * which needs a fresh mapping object, and the hypervisor requires the
>  	 * min-pages memcache even when the install allocates nothing.
>  	 */
>  	memcache = get_mmu_memcache(s2fd->vcpu);
> -	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
> -			    kvm_is_write_fault(s2fd->vcpu)) ||
> +	if (!perm_fault || memslot_is_logging(s2fd->memslot) ||
>  	    is_protected_kvm_enabled()) {

[Severity: High]
This is a pre-existing issue, but does skipping the memcache top-up here
leave the host vulnerable to a crash from a malicious nested L1 guest?

If a nested L1 guest establishes a 2MB stage-2 block mapping, and then
maliciously overwrites it with a 4KB page table descriptor without issuing
a TLBI (an architectural Break-Before-Make violation), an L2 guest can
trigger a permission fault on the stale 2MB shadow mapping.

Since KVM optimizes permission faults by assuming they only relax
permissions, the memcache top-up is skipped here.

However, in kvm_s2_fault_map(), if the mapping size and fault granule
don't match:

arch/arm64/kvm/mmu.c:kvm_s2_fault_map()
    ...
	} else {
		ret = KVM_PGT_FN(kvm_pgtable_stage2_map)(pgt, gfn_to_gpa(gfn), mapping_size,
							 __pfn_to_phys(pfn), prot,
							 memcache, flags);
	}
    ...

kvm_pgtable_stage2_map() will attempt to split the block, which requires
a new page table allocation. Because the cache wasn't topped up,
kvm_mmu_memory_cache_alloc() will hit WARN_ON(!mc->nobjs) and fall back
to a GFP_ATOMIC allocation. If this atomic allocation fails under memory
pressure, it hits BUG_ON(!p) and crashes the host.

>  		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
>  		if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717130317.1953574-1-fuad.tabba@linux.dev?part=3