MIME-Version: 1.0
References: <AM6PR09MB352386B26E3F632D6FE2FDDDD2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
 <CAKv+Gu8000sDAX89=cre2uZ07vJC86FVjcoKagyk_Qqyrkbytw@mail.gmail.com> <AM6PR09MB3523E3B73CFCC6087C6981F1D2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
In-Reply-To: <AM6PR09MB3523E3B73CFCC6087C6981F1D2250@AM6PR09MB3523.eurprd09.prod.outlook.com>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Wed, 17 Apr 2019 13:51:03 -0700
Message-ID: <CAKv+Gu-KFswn=y19YmwhmUfEG9N5T3isRyvzLsvXOM=5Z5vYtw@mail.gmail.com>
Subject: Re: Question regarding crypto scatterlists / testmgr
To:     Pascal Van Leeuwen <pvanleeuwen@insidesecure.com>
Cc:     "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        Eric Biggers <ebiggers@kernel.org>,
        Herbert Xu <herbert@gondor.apana.org.au>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Wed, 17 Apr 2019 at 13:49, Pascal Van Leeuwen
<pvanleeuwen@insidesecure.com> wrote:
>
> > > I'm trying to fix the inside-secure driver to pass all testmgr
> > > tests and I have one final issue remaining with the AEAD ciphers.
> > > As it was not clear at all what the exact problem was, I spent
> > > some time reverse engineering testmgr and I got the distinct
> > > impression that it is using scatter particles that cross page
> > > boundaries. On purpose, even.
> > >
> > > While the inside-secure driver is built on the premise that
> > > scatter particles are continuous in device space. As I can't
> > > think of any reason why you would want to scatter/gather other
> > > than to handle virtual-to-physical address translation ...
> > > In any case, this should affect all other other operations as
> > > well, but maybe those just got "lucky" by getting particles
> > > that were still contiguous in device space, despite the page
> > > crossing (to *really* verify this, you would have to fully
> > > randomize your page allocation!)
> > >
> > > Anyway, assuming that I *should* be able to handle particles
> > > that are *not* contiguous in device space, then there should
> > > probably already exist some function in the kernel API that
> > > converts a scatterlist with non-contiguous particles into a
> > > scatterlist with contiguous particles, taking into account the
> > > presence of an IOMMU? Considering pretty much every device
> > > driver would need to do that?
> > > Does anyone know which function(s) to use for that?
> > >
> >
> > Hello Pascal,
> >
> > Scatterlists are made up of struct page/offset tuples, and so they
> > should map transparently onto physical ranges.
> >
> > It looks like the AEAD skcipher walk API lacks a *_async() variant
> > setting the SKCIPHER_WALK_PHYS bit, like we have for the ordinary
> > block ciphers. Plumbing that into crypto/skcipher.c should be rather
> > straight-forward.
> >
> > --
> > Ard.
> Ard,
>
>
> Am I reading this correctly as "your driver should indeed expect
> those particles to be contiguous, but there is some problem in some
> other location in the API causing this to not always be the case"?
>

Indeed.

> I took a quick peek at skcipher.c, which is a design unfamiliar to
> me, but the first thing that strikes me is that there is an
> "skcipher_walk_async", that indeed sets this SKCIPHER_WALK_PHYS bit,
> but no matching "skcipher_walk_aead_async" function that I would sort
> of expect doing a similar thing for AEAD ciphers ...
>

That is precisely my point.