linux-kernel - Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1507035334.29211.105.camel@infradead.org>
Date:   Tue, 03 Oct 2017 13:55:34 +0100
From:   David Woodhouse <dwmw2@...radead.org>
To:     Robin Murphy <robin.murphy@....com>, joro@...tes.org
Cc:     ashok.raj@...el.com, leedom@...lsio.com, Harsh@...lsio.com,
        herbert@...dor.apana.org.au, iommu@...ts.linux-foundation.org,
        linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling

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.

Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (4938 bytes)