Re: [PATCH v9 22/25] iommu/arm-smmu-v3-kvm: Shadow the CPU stage-2 page table

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iommu/arm-smmu-v3: incorrect recovery from -EEXIST in smmu_host_stage2_idmap leaves holes in memory
--

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

iommu/arm-smmu-v3-kvm: Shadow the CPU stage-2 page table

This commit mirrors changes to the host stage-2 page table into an identity
mapped stage-2 for the SMMUv3. It handles memory mapped with PAGE_SIZE and
MMIO mapped with the largest block that fits.

> diff --git a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
> index 943ffef0bb4aa..75b2b1477a4fa 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/pkvm/arm-smmu-v3-hyp.c
[ ... ]
> @@ -1011,8 +1070,100 @@ static bool smmu_dabt_handler(struct user_pt_regs *regs, u64 esr, u64 addr)
[ ... ]
> +static int smmu_host_stage2_idmap(phys_addr_t start, phys_addr_t end, int prot)
> +{
> +	size_t pgsize = PAGE_SIZE, pgcount, size;
> +	struct io_pgtable *pgtable = idmap_pgtable;
> +	int ret = 0;
> +
> +	end = min(end, BIT(pgtable->cfg.oas));
> +	if (start >= end)
> +		return 0;
> +
> +	size = end - start;
> +	if (prot) {
> +		size_t mapped;
> +
> +		if (!(prot & IOMMU_MMIO))
> +			prot |= IOMMU_CACHE;
> +
> +		while (size) {
> +			mapped = 0;
[ ... ]
> +			if (prot & IOMMU_MMIO)
> +				pgsize = smmu_pgsize_idmap(size, start, pgtable->cfg.pgsize_bitmap);
> +
> +			pgcount = size / pgsize;
> +			ret = pgtable->ops.map_pages(&pgtable->ops, start, start,
> +						     pgsize, pgcount, prot, 0, &mapped);
> +			size -= mapped;
> +			start += mapped;
> +
> +			if (ret == -EEXIST) {
> +				/*
> +				 * It is possible to get EEXIST when a VM dies with pages
> +				 * in a shared state.
> +				 */
> +				ret = 0;
> +				size -= pgsize;
> +				start += pgsize;
> +				continue;
> +			}

[Severity: High]
Does this error handling skip valid pages when a collision occurs later in
the batch?

The io-pgtable-arm backend implementation of map_pages validates all entries
in the batch before writing any PTEs:

drivers/iommu/io-pgtable-arm.c:arm_lpae_init_pte() {
	for (i = 0; i < num_entries; i++)
		if (iopte_leaf(ptep[i], lvl, data->iop.fmt)) {
			/* We require an unmap first */
			WARN_ON(!(data->iop.cfg.quirks & IO_PGTABLE_QUIRK_NO_WARN));
			return -EEXIST;
		}
}

If a collision is found at any index (for example, the 5th page of the batch),
map_pages returns -EEXIST immediately without mapping anything, leaving mapped
at 0.

Because mapped is 0, this code assumes the very first page of the batch caused
the collision, skips it by doing start += pgsize, and retries the loop. This
process repeatedly skips earlier non-colliding pages without mapping them,
leaving holes in the SMMU stage 2 identity map and causing translation faults
during host DMA.

> +			if (!mapped || ret)
> +				break;
> +		}

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