On Tue, Sep 26, 2017 at 9:06 AM, Casey Leedom <leedom@xxxxxxxxxxx> wrote: > | From: Robin Murphy <robin.murphy@xxxxxxx> > | 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.