From:   Pascal Van Leeuwen <pvanleeuwen@insidesecure.com>
To:     Eric Biggers <ebiggers@kernel.org>
CC:     "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
Subject: RE: Question regarding crypto scatterlists / testmgr
Thread-Topic: Question regarding crypto scatterlists / testmgr
Thread-Index: AdT1UrNnwXz5dYegQq+/zS0mWvhdgAACM8MAAAEghoAAANhrYA==
Date:   Wed, 17 Apr 2019 21:22:27 +0000
Message-ID: <AM6PR09MB35238364D49784A702B2B064D2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
References: <AM6PR09MB352386B26E3F632D6FE2FDDDD2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
 <20190417202407.GA96242@gmail.com>
 <AM6PR09MB3523FD88FE64E10B78B555F4D2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
In-Reply-To: <AM6PR09MB3523FD88FE64E10B78B555F4D2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: insidesecure.com does not
 designate permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 62609846-e802-4c32-b719-08d6c37acabc
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2019 21:22:27.3174
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3c07df58-7760-4e85-afd5-84803eac70ce
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR09MB3238
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

> > Indeed, since v5.1, testmgr tests scatterlist elements that cross a
> > page.
> > However, the pages are guaranteed to be *physically* contiguous.
> Does
> > dma_map_sg() not handle this?
> >
> I'm not entirely sure and the API documentation is not particularly
> clear on *what* dma_map_sg() actually does, but I highly doubt it
> considering the particle count is only an input parameter (i.e. it
> can't output an increase in particles that would be required).
> So I think it just ensures the pages are actually flushed to memory
> and accessible by the device (in case an IOMMU interferes) and not
> much than that.
>
> In any case, scatter particles to be used by hardware should *not*
> cross any physical page boundaries.
> But also see the thread I had on this with Ard - seems like the crypto
> API already has some mechanism for enforcing this but it's not enabled
> for AEAD ciphers?
>
> >
> > BTW, this isn't just a theoretical case.  Many crypto API users do
> > crypto on
> > kmalloced buffers, and those can cross a page boundary, especially if
> > they are
> > large.  All software crypto algorithms handle this case.
> >
> Software sits behind the CPU's MMU and sees virtual memory as
> contiguous. It does not need to "handle" anything, it gets it for free.
> Hardware does not have that luxury, unless you have a functioning IOMMU
> but that is still pretty rare.
> So for hardware, you need to break down your buffers until individual
> pages and stitch those together. That's the main use case of a scatter
> list and it requires the particles to NOT cross physical pages.
>
> > The fact that these types of issues are just being considered now
> > certainly
> > isn't raising my confidence in the hardware crypto drivers in the
> > kernel...
> >
> Actually, this is *not* a problem with the hardware drivers. It's a
> problem with the API and/or how you are trying to use it. Hardware
> does NOT see the nice contiguous virtual memory that SW sees.
>
> If the driver may expect to receive particles that cross page
> boundaries - if that's the spec - fine, but then it will have to
> break those down into individual pages by itself. However, whomever
> created the inside-secure driver was under the impression that this
> was not supposed to be the case. And I don't know who's right or
> wrong there, but from a side discussion with Ard I got the impression
> that the Crypto API should fix this up before it reaches the driver.
>
Long story short: testmgr appears to be doing nothing wrong AND the
driver appears to be doing nothing wrong, but it seems like there's a
bug in the Crypto API itself with the scatter walk for AEAD ciphers.

Regards,

Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Inside Secure