Re: [PATCH v5 3/6] iommu/dma: Catch scatterlist length overflows

On 17/08/2026 10:56 am, Krzysztof Karas wrote:
> It is possible, when a very large mapping uses only one
> 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 scatterlist
> length field.
> 
> Fixes: 809eac54cdd6 ("iommu/dma: Implement scatterlist segment merging")
> Signed-off-by: Krzysztof Karas <[email protected]>
> ---
> v5 (sashiko review):
>   * Included overflow checking on addition, previously silently
>   omitted inside iova_align arguments.
>   * Added check for zero-mapping via temporary variable comparison.
>   * Added overflows_type on s->length to satisfy 64 bit systems.
> 
>   drivers/iommu/dma-iommu.c | 32 ++++++++++++++++++++++++++++++--
>   1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index 9abaec0703ef..0bd5b13f006f 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -21,6 +21,7 @@
>   #include <linux/iommu-dma.h>
>   #include <linux/iova.h>
>   #include <linux/irq.h>
> +#include <linux/limits.h>
>   #include <linux/list_sort.h>
>   #include <linux/memremap.h>
>   #include <linux/mm.h>
> @@ -1445,6 +1446,7 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>   	for_each_sg(sg, s, nents, i) {
>   		size_t s_iova_off = iova_offset(iovad, s->offset);
>   		size_t s_length = s->length;
> +		size_t s_length_tmp;
>   		size_t pad_len = (mask - iova_len + 1) & mask;
>   
>   		switch (pci_p2pdma_state(&p2pdma_state, dev, sg_page(s))) {
> @@ -1476,7 +1478,22 @@ int iommu_dma_map_sg(struct device *dev, struct scatterlist *sg, int nents,
>   		sg_dma_address(s) = s_iova_off;
>   		sg_dma_len(s) = s_length;
>   		s->offset -= s_iova_off;
> -		s_length = iova_align(iovad, s_length + s_iova_off);
> +
> +		if (check_add_overflow(s_length, s_iova_off, &s_length_tmp)) {
> +			ret = -EOVERFLOW;
> +			goto out_restore_sg;
> +		}

This is unnecessary - s_iova_off will always be <= sg->offset, and since 
the maximum possible segment boundary mask is 32 bits, if sg->offset + 
sg->length would overflow then by definition it must cross a segment 
boundary, so the segment is malformed to begin with and all bets are off.

> +		s_length = iova_align(iovad, s_length_tmp);
> +
> +		if (s_length_tmp != 0 && s_length == 0) {
> +			ret = -EOVERFLOW;
> +			goto out_restore_sg;
> +		}

Similarly, a 0-length segment wouldn't be valid either (consider in the 
simple case it would end up with sg_dma_len() == 0 which means "end of 
mapping")

Thanks,
Robin.

> +
> +		if (overflows_type(s_length, s->length)) {
> +			ret = -EOVERFLOW;
> +			goto out_restore_sg;
> +		}
>   		s->length = s_length;
>   
>   		/*
> @@ -1493,7 +1510,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;
> +			unsigned int new_prev_len;
> +			/*
> +			 * For large mappings spanning multiple GBs we
> +			 * may not be able to fit all needed padding into
> +			 * sg->length.
> +			 */
> +			if (check_add_overflow(prev->length, pad_len, &new_prev_len)) {
> +				ret = -EOVERFLOW;
> +				goto out_restore_sg;
> +			}
> +
> +			prev->length = new_prev_len;
>   			iova_len += pad_len;
>   		}
>