linux-kernel - Re: DMA error when sg->offset value is greater than PAGE

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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 26 Sep 2017 09:10:52 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Casey Leedom <leedom@...lsio.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Harsh Jain <Harsh@...lsio.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        David Woodhouse <David.Woodhouse@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "iommu@...ts.linux-foundation.org" <iommu@...ts.linux-foundation.org>,
        "linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        Michael Werner <werner@...lsio.com>
Subject: Re: DMA error when sg->offset value is greater than PAGE_SIZE in
 Intel IOMMU

On Tue, Sep 26, 2017 at 9:06 AM, Casey Leedom <leedom@...lsio.com> wrote:
> | From: Robin Murphy <robin.murphy@....com>
> | Sent: Tuesday, September 26, 2017 7:22 AM
>
> |
> | On 26/09/17 13:21, Harsh Jain wrote:
> | > Find attached new set of log. After repeated tries it panics.
> |
> | Thanks, that makes things a bit clearer - looks like fixing the physical
> | address/pteval calculation to not be off by a page in one direction wasn't
> | helping much because the returned DMA address is actually also off by a
> | page in the other direction, and thus overflowing past the allocated IOVA
> | into whoever else's mapping happened to be there; complete carnage ensues.
> |
> | After another look through the intel_map_sg() path, here's my second
> (still
> | completely untested) guess at a possible fix.
> |
> | Robin.
> |
> | ----->8-----
> | diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> | index 6784a05dd6b2..d7f7def81613 100644
> | --- a/drivers/iommu/intel-iommu.c
> | +++ b/drivers/iommu/intel-iommu.c
> | @@ -2254,10 +2254,12 @@ static int __domain_mapping(struct dmar_domain
> *domain, unsigned long iov_pfn,
> |                  uint64_t tmp;
> |
> |                  if (!sg_res) {
> | +                       size_t off = sg->offset & ~PAGE_MASK;
> | +
> |                          sg_res = aligned_nrpages(sg->offset, sg->length);
> | -                       sg->dma_address = ((dma_addr_t)iov_pfn <<
> VTD_PAGE_SHIFT) + sg->offset;
> | +                       sg->dma_address = ((dma_addr_t)iov_pfn <<
> VTD_PAGE_SHIFT) + off;
> |                          sg->dma_length = sg->length;
> | -                       pteval = page_to_phys(sg_page(sg)) | prot;
> | +                       pteval = (page_to_phys(sg_page(sg)) + sg->offset -
> off) | prot;
> |                          phys_pfn = pteval >> VTD_PAGE_SHIFT;
> |                  }
>
>   Thanks Robin.  And thanks Harsh for sending the detailed trace logs.  I'll
> see if I can get this tested today.  Harsh is probably headed towards bed,
> but there may be sufficiently good instructions in our internal bug system
> to reproduce the issue.
>
>   Regardless, it seems that you agree that there's an issue with the Intel
> I/O MMU support code with regard to the legal values which a (struct
> scatterlist) can take on?  I still can't find any documentation for this
> and, personally, I'm a bit baffled by a Page-oriented Scatter/Gather List
> representation where [Offset, Offset+Length) can reside outside the Page.

Consider the case where the page represents a huge page, then an
offset greater than PAGE_SIZE (up to HPAGE_SIZE) makes sense.