Re: [PATCH 1/2] crypto: talitos - Work around SEC6 ERRATA (AES-CTR mode data size error)

[Christophe Leroy]

Le 21/01/2021 à 11:02, Ard Biesheuvel a écrit :
> On Thu, 21 Jan 2021 at 10:54, Christophe Leroy
> <christophe.leroy@xxxxxxxxxx> wrote:
> > Le 21/01/2021 à 08:31, Ard Biesheuvel a écrit :
> > > On Thu, 21 Jan 2021 at 06:35, Christophe Leroy
> > > <christophe.leroy@xxxxxxxxxx> wrote:
> > > > Le 20/01/2021 à 23:23, Ard Biesheuvel a écrit :
> > > > > On Wed, 20 Jan 2021 at 19:59, Christophe Leroy
> > > > > <christophe.leroy@xxxxxxxxxx> wrote:
> > > > > > Talitos Security Engine AESU considers any input
> > > > > > data size that is not a multiple of 16 bytes to be an error.
> > > > > > This is not a problem in general, except for Counter mode
> > > > > > that is a stream cipher and can have an input of any size.
> > > > > >
> > > > > > Test Manager for ctr(aes) fails on 4th test vector which has
> > > > > > a length of 499 while all previous vectors which have a 16 bytes
> > > > > > multiple length succeed.
> > > > > >
> > > > > > As suggested by Freescale, round up the input data length to the
> > > > > > nearest 16 bytes.
> > > > > >
> > > > > > Fixes: 5e75ae1b3cef ("crypto: talitos - add new crypto modes")
> > > > > > Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> > > > >
> > > > > Doesn't this cause the hardware to write outside the given buffer?
> > > >
> > > >
> > > > Only the input length is modified. Not the output length.
> > > >
> > > > The ERRATA says:
> > > >
> > > > The input data length (in the descriptor) can be rounded up to the nearest 16B. Set the
> > > > data-in length (in the descriptor) to include X bytes of data beyond the payload. Set the
> > > > data-out length to only output the relevant payload (don't need to output the padding).
> > > > SEC reads from memory are not destructive, so the extra bytes included in the AES-CTR
> > > > operation can be whatever bytes are contiguously trailing the payload.
> > >
> > > So what happens if the input is not 16 byte aligned, and rounding it
> > > up causes it to extend across a page boundary into a page that is not
> > > mapped by the IOMMU/SMMU?
> >
> > What is the IOMMU/SMMU ?
> >
> > The mpc8xx, mpc82xx and mpc83xx which embed the Talitos Security Engine don't have such thing, the
> > security engine uses DMA and has direct access to the memory bus for reading and writing.
>
> OK, good. So the only case where this could break is when the DMA
> access spills over into a page that does not exist, and I suppose this
> could only happen if the transfer involves a buffer located at the
> very top of DRAM, right?

Right.

Christophe