On Thu, 2017-09-28 at 15:14 +0100, Robin Murphy wrote:
> The intel-iommu DMA ops fail to correctly handle scatterlists where
> sg->offset is greater than PAGE_SIZE - the IOVA allocation is computed
> appropriately based on the page-aligned portion of the offset, but the
> mapping is set up relative to sg->page, which means it fails to actually
> cover the whole buffer (and in the worst case doesn't cover it at all):
> 
>     (sg->dma_address + sg->dma_len) ----+
>     sg->dma_address ---------+          |
>     iov_pfn------+           |          |
>                  |           |          |
>                  v           v          v
> iova:   a        b        c        d        e        f
>         |--------|--------|--------|--------|--------|
>                           <...calculated....>
>                  [_____mapped______]
> pfn:    0        1        2        3        4        5
>         |--------|--------|--------|--------|--------|
>                  ^           ^          ^
>                  |           |          |
>     sg->page ----+           |          |
>     sg->offset --------------+          |
>     (sg->offset + sg->length) ----------+

I'd still dearly love to see some clear documentation of what it means
for sg->offset to be outside the page referenced by sg->page.

Or is it really not "outside", and it's *only* valid for the offset to
be > PAGE_OFFSET when it's a huge page, so we can check that with a
BUG_ON() ? 

In particular, I'd like to know what is intended in the Xen PV case,
where there isn't a straight correspondence between pfn and mfn. Is the
out-of-range sg->offset intended to refer to the next *pfn* after sg-
>page, or to the next *mfn* after sg->page? 

I confess I've only followed this thread vaguely, but I haven't seen a
*coherent* explanation except in the huge page case (in which case I
want to see that BUG_ON in the patch) of why this isn't just totally
bogus.