Re: [PATCH] KVM: arm64: skip pKVM cache flushes for non cacheable mappings

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Handling a permission fault that upgrades a mapping to a huge page skips memcache top-up, leading to a NULL pointer dereference in `pkvm_pgtable_stage2_map()`.
--

commit d1dad1f606a114503ffa8d910f3c79c232636e5c
Author: Bradley Morgan <include@grrlz.net>

KVM: arm64: skip pKVM cache flushes for non cacheable mappings

This patch adds a cacheable flag to pKVM mappings to avoid flushing
non-cacheable stage 2 mappings. It updates pkvm_pgtable_stage2_map to
record this attribute and pkvm_pgtable_stage2_flush to skip flushing.

[ ... ]

> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> index 053e4f733e4be..efede6b5640e0 100644
> --- a/arch/arm64/kvm/pkvm.c
> +++ b/arch/arm64/kvm/pkvm.c
> @@ -473,6 +473,8 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable *pgt, u64 addr, u64 size,
>  	mapping->gfn = gfn;
>  	mapping->pfn = pfn;
>  	mapping->nr_pages = size / PAGE_SIZE;

[Severity: Critical]
This is a pre-existing issue, but could this lead to a NULL pointer
dereference if the memcache wasn't topped up?

In arch/arm64/kvm/mmu.c:user_mem_abort(), topup_mmu_memcache() is skipped
for permission faults when dirty logging is not active:

	memcache = get_mmu_memcache(s2fd->vcpu);
	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
			    kvm_is_write_fault(s2fd->vcpu))) {
		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
		if (ret)
			return ret;
	}

If a guest triggers a permission fault on an address backed by a transparent
huge page (where the existing stage-2 mapping is a page-level mapping),
kvm_s2_fault_map() will upgrade the mapping to a block and call
pkvm_pgtable_stage2_map().

For non-protected VMs on a pKVM host, pkvm_pgtable_stage2_map() then unshares
the old mapping, sets mapping = NULL, and swaps it with cache->mapping:

	swap(mapping, cache->mapping);
	mapping->gfn = gfn;

Because the top-up was skipped, cache->mapping is NULL, meaning mapping->gfn
dereferences NULL, allowing an untrusted guest to crash the host.

> +	mapping->cacheable = !(prot & (KVM_PGTABLE_PROT_DEVICE |
> +				       KVM_PGTABLE_PROT_NORMAL_NC));
>  	pkvm_mapping_insert(mapping, &pgt->pkvm_mappings);
>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623160339.15143-1-include@grrlz.net?part=1