On 2026-07-03 9:35 pm, Jason Gunthorpe wrote:
On Fri, Jul 03, 2026 at 07:58:32PM +0100, Robin Murphy wrote:
On 03/07/2026 5:22 pm, Jason Gunthorpe wrote:
On Wed, Jul 01, 2026 at 10:44:37AM +0000, 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.
Urk, this is unfortunate, it means we cannot map certain kinds of
scatterlists? Meaning there is a condition that makes a scatterlist
ill formed?
This seems like something that needs to be more clearly documented and
we need to ensure at least the common scatterlist builders don't hit
it..
Well, it's taken 10 years to be caught by a test which seemingly expects the
mapping of a single absurdly giant scatterlist to fail anyway,
If your server has 0.5TB of ram a 4G IO isn't actually that large.
Randomly getting a few contiguous 1G hugetlbfs pages is not even that
unlikely. Something like FSDAX has a very high chance of getting high
contiguity pages in files.
Sure, but how many servers had 0.5TB of RAM in 2015? And how many of
those were running the arm64 DMA mapping code? As I said, both this
merging logic and the iommu_map_sg() interface itself were essentially
written to support media buffers on Android phones which didn't even
have 4GB of RAM in total. Yes, things have moved on by now, but
correspondingly it has also been decided that the ~30-year-old
scatterlist design also doesn't scale to modern use-cases anyway, and is
being replaced, so limitations of a "legacy" API that don't have any
meaningful impact to its existing users are hardly something to panic
about. If DRM does want to be able to *reliably* map massive amounts of
RAM then it can adopt the new IOMMU API, for this and all the other
reasons that that new API was promised to be "better".
So I wouldn't be quite so dismissive that this is not something a real
user can hit.
I'm not being dismissive - clearly it can be hit. My point is that
anyone who *does* hit it can only expect it to fail (as indeed this
particular IGT test seems to), because it has never worked. You cannot
encode a 4GB scatterlist segment, because it overflows UINT_MAX. And if
you did try to bodge it and pass a UINT_MAX length segment to
iommu_map_sg() then that will also fail because it's not aligned to an
IOMMU page size. It's purely the matter of *how* exactly it fails which
could do with fixing.
here, but I suspect this is likely just regular iova_granule rounding
overflowing when the segment boundary is the maximum 4GB, since the largest
representable segment length is 4GB - 1.
It looks like the iommu_dma_map_sg() algorithm only works reliably if
the scatterlist entry size is less than UINT_MAX/2, otherwise it can
risk overflowing when it pads.
AFAICS, the overflow can only happen with a 4GB boundary mask, and for
two conditions:
- rounding up the current segment, when s->length + s->offset > 4GB -
iova_granule (but still <= 4GB otherwise it's bogus anyway)
- padding the previous segment in the case where the current segment
would otherwise cross the next boundary, when prev->offset = 0 and
prev->length + s->length >= 4GB
So yes, limiting any individual segment to <=2GB would end up avoiding
both those conditions, but it would also impact plenty of cases that
*do* currently work fine, e.g. 1GB+3GB+3GB. The limitation is really
that you can't have two consecutive segments where the first starts
exactly on a 4GB boundary and the sum of both their sizes >=4GB.
API wise I expect any arbitary input to sg_alloc_table_from_pages() to
result in a scatterlist that iommu_dma_map_sg() will map. This
patch highlights there are cornere cases where that isn't true, it
should be fixed..
Technically sg_alloc_table_from_pages() carries no such assumption, only
sg_alloc_table_from_pages_segment() (or __sg_alloc_table_from_pages())
with the correct dma_seg_boundary value for the given device. But even
then in the worst case, they should still end up splitting segments at
4GB-PAGE_SIZE due to the fundamental int limitation, and so only be at
risk of putting two such segments back-to-back.
I agree we shouldn't overcomplicate iommu_dma_map_sg(), so the
simplest fix is to introduce a SG_MAX_LENGTH set to UINT_MAX/2,
justified by the logic in iommu_dma_map_sg(). Fixup the core sg_alloc
code to respect that. WARN_ON in iommu_dma_map_sg() if a malformed
scatterlist entry is presented. Add a WARN_ON under DMA debugging
kconfig as well for the physical path.
Again, it's not "malformed", it's just an edge case of certain
otherwise-valid scatterlist layouts that are not supported in this one
DMA API implementation. Nothing in the DMA API ever guarantees that any
particular mapping must succeed. Furthermore I don't see that anyone's
asking for this to actually be supported, just to fail cleanly and
correctly without inadvertently corrupting state.
Thanks,
Robin.
