On 02/07/2026 7:22 am, Krzysztof Karas wrote:
Hi Robin,
thanks for a quick response!
On 2026-07-01 at 12:59:29 +0100, Robin Murphy wrote:
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")
Okay, thanks for pinpointing this commit, I'll add the tag in
the next version of this series.
Signed-off-by: Krzysztof Karas <[email protected]>
---
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: ...").
I'll be making another version anyway, so I can remove the
"else" and change the title :)
Cool :)
Either way,
Reviewed-by: Robin Murphy <[email protected]>
Thanks!
would this r-b still hold after above minor changes, or would
you prefer to have one final look at the finished change and
then decide whether you give your r-b?
Indeed if it's just those tweaks then feel free to keep it, that's
usually what I mean by "either way".
Cheers,
Robin.
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;
