Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S937389AbdIZQK5 (ORCPT ); Tue, 26 Sep 2017 12:10:57 -0400 Received: from mail-oi0-f53.google.com ([209.85.218.53]:53775 "EHLO mail-oi0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S936129AbdIZQKy (ORCPT ); Tue, 26 Sep 2017 12:10:54 -0400 X-Google-Smtp-Source: AOwi7QD7FIVItneKIITaSSd6VdcXUXtC1RZip5VRVABjlgIkpoA7jERxpu4kfk9+5x7uBtGGk7PzOs+03QkP3O6m2Nc= MIME-Version: 1.0 In-Reply-To: References: <20170920080151.GA3348@gondor.apana.org.au> <26992a1e-edb3-ed78-ce8e-31e0739d75f4@arm.com> <20170925155430.GB131920@otc-nc-03> <6d2af675-7b97-6eaf-4daa-d7bf80a05923@chelsio.com> <437a9bd8-d4d6-22ca-1a64-1a3e73f1101a@arm.com> From: Dan Williams Date: Tue, 26 Sep 2017 09:10:52 -0700 Message-ID: Subject: Re: DMA error when sg->offset value is greater than PAGE_SIZE in Intel IOMMU To: Casey Leedom Cc: Robin Murphy , Harsh Jain , "Raj, Ashok" , Herbert Xu , David Woodhouse , "linux-kernel@vger.kernel.org" , "iommu@lists.linux-foundation.org" , "linux-crypto@vger.kernel.org" , "dwmw2@infradead.org" , Michael Werner Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2651 Lines: 57 On Tue, Sep 26, 2017 at 9:06 AM, Casey Leedom wrote: > | From: Robin Murphy > | 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.