Re: [PATCH v8 01/25] KVM: arm64: Donate MMIO to the hypervisor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: arm64: resource leak and memory attribute mismatch on MMIO donation error path
--

commit 96c49002ec46fcffdec0cb75c633309cd148cad6
Author: Mostafa Saleh <smostafa@google.com>

KVM: arm64: Donate MMIO to the hypervisor

This commit introduces functions to donate MMIO memory between the host and
hypervisor. It enables IOMMU hypervisor drivers to protect and access the
MMIO of IOMMUs by mapping the MMIO range in the hypervisor's private range
and unmapping it from the host stage-2.

> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c

[ ... ]

> +int __pkvm_host_donate_hyp_mmio(u64 pfn, u64 nr_pages, unsigned long *haddr)
> +{
> +	u64 phys = hyp_pfn_to_phys(pfn);
> +	u64 size = PAGE_SIZE * nr_pages;
> +	kvm_pte_t pte;
> +	u64 offset;
> +	int ret;
> +
> +	/* Only before de-privilege. */
> +	if (static_branch_unlikely(&kvm_protected_mode_initialized))
> +		return -EPERM;
> +
> +	if (!pfn_range_is_valid(pfn, nr_pages))
> +		return -EINVAL;
> +
> +	ret = __pkvm_create_private_mapping(phys, size, PAGE_HYP_DEVICE, haddr);
> +	if (ret)
> +		return ret;
> +
> +	host_lock_component();
> +	for (offset = 0; offset < size; offset += PAGE_SIZE) {
> +		if (addr_is_memory(phys + offset)) {
> +			ret = -EINVAL;
> +			goto unlock;
> +		}
> +		ret = kvm_pgtable_get_leaf(&host_mmu.pgt, phys + offset, &pte, NULL);
> +		if (ret)
> +			goto unlock;
> +		if (pte && !kvm_pte_valid(pte)) {
> +			ret = -EPERM;
> +			goto unlock;
> +		}
> +	}

[Severity: Medium]
Does this leak the hypervisor private mapping and leave a mismatched memory
attribute if validation fails?

In __pkvm_host_donate_hyp_mmio(), the PAGE_HYP_DEVICE mapping is created
before the physical address is validated against addr_is_memory() or the
page tables. If the validation fails, the function exits via the unlock
label and returns an error without rolling back the private mapping.

If the host firmware provides an invalid physical address (like a RAM
address) for MMIO donation, this leaves the virtual address space leaked
in EL2. Additionally, keeping RAM mapped as PAGE_HYP_DEVICE creates a
mismatched memory attribute (Device vs Normal) for RAM in the hypervisor
translation regime, which can cause unpredictable behavior.

> +	/*
> +	 * We set HYP as the owner of the MMIO pages in the host stage-2, for:
> +	 * - host aborts: host_stage2_adjust_range() would fail for invalid non zero PTEs.
> +	 * - recycle under memory pressure: host_stage2_unmap_dev_all() would call
> +	 *   kvm_pgtable_stage2_unmap() which will not clear non zero invalid ptes (counted).
> +	 * - other MMIO donation: Would fail as we check that the PTE is valid or empty.
> +	 */
> +	ret = host_stage2_try(kvm_pgtable_stage2_annotate, &host_mmu.pgt,
> +			      phys, size, &host_s2_pool,
> +			      KVM_HOST_INVALID_PTE_TYPE_DONATION,
> +			      FIELD_PREP(KVM_HOST_DONATION_PTE_OWNER_MASK, PKVM_ID_HYP));
> +unlock:
> +	host_unlock_component();
> +	return ret;
> +}

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