Re: [PATCH v6 1/2] iommu: use resource_size_t for reserved region length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] vfio/type1: truncation of region->length in vfio_find_dma() on 32-bit LPAE
--

commit 0e9e6ab1dd5582c6a157191dbe1e262f13c108a9
Author: Vikash Garodia <vikash.garodia@oss.qualcomm.com>

iommu: use resource_size_t for reserved region length

This commit widens struct iommu_resv_region.length and related helper
parameters from size_t to resource_size_t so that reserved regions of 4GiB
or larger are not truncated on 32-bit systems with LPAE.

[ ... ]

> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index ef5043752a365..1d8f9381a358e 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -306,7 +306,7 @@ enum iommu_resv_type {
>  struct iommu_resv_region {
>  	struct list_head	list;
>  	phys_addr_t		start;
> -	size_t			length;
> +	resource_size_t		length;

[Severity: Medium]
Now that region->length can hold values of 4GiB or larger on 32-bit LPAE
systems, could it still get truncated when passed to vfio_find_dma() via
vfio_iommu_type1_attach_group() -> vfio_iommu_resv_conflict()?

In drivers/vfio/vfio_iommu_type1.c, vfio_iommu_resv_conflict() passes
region->length directly to vfio_find_dma():

drivers/vfio/vfio_iommu_type1.c:vfio_iommu_resv_conflict() {
	list_for_each_entry(region, resv_regions, list) {
		if (region->type == IOMMU_RESV_DIRECT_RELAXABLE)
			continue;

		if (vfio_find_dma(iommu, region->start, region->length))
			return true;
	}
}

However, vfio_find_dma() still takes size as a 32-bit size_t on 32-bit LPAE
systems:

drivers/vfio/vfio_iommu_type1.c:vfio_find_dma() {
static struct vfio_dma *vfio_find_dma(struct vfio_iommu *iommu,
				      dma_addr_t start, size_t size)
{
	struct rb_node *node = iommu->dma_list.rb_node;

	WARN_ON(!size);

	while (node) {
		struct vfio_dma *dma = rb_entry(node, struct vfio_dma, node);

		if (start + size - 1 < dma->iova)
...
}

If an IOMMU group with a reserved region of 4GiB or larger is attached to a
VFIO container on a 32-bit LPAE system, region->length will be truncated to
32 bits. When region->length is a multiple of 4GiB, size becomes 0,
triggering WARN_ON(!size) and miscomputing start + size - 1; for other
sizes >= 4GiB, the upper portion of the reserved region will not be checked
for conflicting DMA mappings.

Should the size parameter of vfio_find_dma() (or a dedicated range helper)
also be widened to resource_size_t or dma_addr_t?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-add_iommu_ranges-v6-0-361f3921a133@oss.qualcomm.com?part=1