Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187AbdI0Rki (ORCPT ); Wed, 27 Sep 2017 13:40:38 -0400 Received: from mga11.intel.com ([192.55.52.93]:45334 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751938AbdI0Rkf (ORCPT ); Wed, 27 Sep 2017 13:40:35 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,445,1500966000"; d="scan'208";a="1176394860" Date: Wed, 27 Sep 2017 07:48:48 -0700 From: "Raj, Ashok" To: Robin Murphy Cc: Casey Leedom , Dan Williams , Harsh Jain , Herbert Xu , "linux-kernel@vger.kernel.org" , "iommu@lists.linux-foundation.org" , "linux-crypto@vger.kernel.org" , "dwmw2@infradead.org" , Michael Werner , nd@arm.com, Ashok Raj Subject: Re: DMA error when sg->offset value is greater than PAGE_SIZE in Intel IOMMU Message-ID: <20170927144847.GA95654@otc-nc-03> References: <20170925155430.GB131920@otc-nc-03> <6d2af675-7b97-6eaf-4daa-d7bf80a05923@chelsio.com> <437a9bd8-d4d6-22ca-1a64-1a3e73f1101a@arm.com> <20170927181802.3dcd7efb@m750.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170927181802.3dcd7efb@m750.lan> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3084 Lines: 75 Hi Robin On Wed, Sep 27, 2017 at 06:18:02PM +0100, Robin Murphy wrote: > On Wed, 27 Sep 2017 16:31:04 +0000 > Casey Leedom wrote: > > > | From: Dan Williams > > | Sent: Tuesday, September 26, 2017 9:10 AM > > | > > | On Tue, Sep 26, 2017 at 9:06 AM, Casey Leedom > > wrote: | > | From: Robin Murphy > > | > | Sent: Tuesday, September 26, 2017 7:22 AM > > | > |... > > | > ... > > | > 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. > > > > Okay, but whatever the underlaying Page Size is, should [Offset, > > Offset+Length) completely reside within the referenced Page? I'm just > > trying to understand the Invariance Conditions which are assumed by > > all of the code which processes Scatter/gather Lists ... > > From my experience, in general terms each scatterlist segment > represents some contiguous quantity of pages, of which sg->page is the > first, while sg->length and sg->offset describe the specific bounds of > that segment's data. As such, the length may certainly (and frequently > does) exceed PAGE_SIZE; for the offset, it's unlikely that the producer > would initially construct one greater than PAGE_SIZE instead of just > pointing sg->page further forward, but it seems reasonable for it to > come about if some intermediate subsystem is processing an existing > list in-place (as seems to be the case with crypto here). > > My opinion is that this may be a slightly unusual case, but I would > not consider it an illegal one. I think most DMA mapping > implementations would handle it whether intentionally or not. In this specific case, it appears that scatterwalk_ffwd()->sg_set_page() sg_set_page(dst, sg_page(src), src->length - len, src->offset + len); and static inline void sg_set_page(struct scatterlist *sg, struct page *page, unsigned int len, unsigned int offset) { sg_assign_page(sg, page); sg->offset = offset; sg->length = len; } The src->offset + len seems to be the culprit putting it past the page. Looks like in the cases when it breaks, the offset is already towards the end of page.. and adding the len, puts it over the limit. When dealing with the offset > PAGE_SIZE, is the expectation you have another additional entry for sgl? for e.g. if sg->page = X, and offset=4092. and len = 16. Since IOMMU only understands 4K pages this last entry needs to be adjusted? I'm not sure if the offset+len is a buffer overflow situation or just trips IOMMU. Cheers, Ashok the scatter gather list, should we