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

[Mostafa Saleh <smostafa@google.com>]

On Tue, Sep 22, 2026 at 01:29:15PM +0000, sashiko-bot@kernel.org wrote:
> 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.

Yes, in that case the private mapping is leaked, as there is no function
to remove private mappings. This a pattern in the hypervisor in similar
error cases during init.

> 
> 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.
> 

pKVM has to trust the firmware, otherwise much bigger problems can
happen. So this check is mainly added for very unlikely conditions
(firmware is really busted or during development)

Thanks,
Mostafa

> > +	/*
> > +	 * 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