Re: [PATCH v2 3/3] drivers/iommu: Catch scatterlist length overflows

[Robin Murphy <robin.murphy@arm.com>]

On 01/07/2026 11:44 am, Krzysztof Karas wrote:
> It is possible, when a very large mapping uses a single
> scatterlist, that padding overflows scatterlist's length field.
> This results in:
>   1) silently wrapping the value
>   2) smaller than desired mappings produced by iommu_map_sg
>   3) leaving mapped bytes in memory (no iommu_unmap)
> 
> Address this issue by adding overflow detection for previous
> scatterlist length field.

Awesome, thanks for figuring it out! Looks like this must date all the 
way back:

Fixes: 809eac54cdd6 ("iommu/dma: Implement scatterlist segment merging")

> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
> ---
> v2:
>   * Address overflows instead of unmapping erroneously mapped
>   memory (Robin).
>   * Put this patch last for easier reproduction of the issue.
> 
>   drivers/iommu/dma-iommu.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 9abaec0703ef..c403057577df 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -1493,8 +1493,18 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>   		 *   time through here (i.e. before it has a meaningful value).
>   		 */
>   		if (pad_len && pad_len < s_length - 1) {
> -			prev->length += pad_len;
> -			iova_len += pad_len;
> +			if (overflows_type(prev->length + pad_len, prev->length)) {
> +				/*
> +				 * For large mappings spanning multiple GBs we
> +				 * may not be able to fit all needed padding into
> +				 * sg->length.
> +				 */
> +				ret = -EOVERFLOW;
> +				goto out_restore_sg;
> +			} else {

Nit: we don't really need an "else" after a goto, but it's hardly a big 
deal (however if you did want to respin, note also that the preferred 
title tag here is "iommu/dma: ..."). Either way,

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

I'd imagine Joerg can take this as an IOMMU fix, but FWIW if you did 
want an ack to take it through drm-fixes to keep it with the i915 
patches, I wouldn't foresee any significant risk of conflicts.

Thanks,
Robin.

> +				prev->length += pad_len;
> +				iova_len += pad_len;
> +			}
>   		}
>   
>   		iova_len += s_length;