mbox series

[v3,0/7] crypto: sun4i-ss: prevent always fallback for ciphers

Message ID 20201116135345.11834-1-clabbe@baylibre.com
Headers show
Series crypto: sun4i-ss: prevent always fallback for ciphers | expand

Message

Corentin Labbe Nov. 16, 2020, 1:53 p.m. UTC
Hello

For help testing on "crypto: sun4i-ss - Fix sparse endianness markers",
I have added "stats" support like other allwinner's crypto drivers.
Seeing stats showed a clear problem, the ciphers function were not used
at all.
This is due to the not-inialized need_fallback which is "init" as true
everytime.
So basicly, since the patch introduced it, this probem hidden some bugs.

This serie fixes all hidden problems, then fix the initialization of
"need_fallback" and then add the stats like other allwinner drivers.

Regards

Changes since v2:
- patch #1: move buf/bufo out of function for reducing stack usage
- patch #4: use writesl()
- patch #6: use IS_ENABLED instead of #ifdef

Changes since v1:
- patch #4 is sufficient to fix BE problem (removed todo)

Corentin Labbe (7):
  crypto: sun4i-ss: linearize buffers content must be kept
  crypto: sun4i-ss: checking sg length is not sufficient
  crypto: sun4i-ss: IV register does not work on A10 and A13
  crypto: sun4i-ss: handle BigEndian for cipher
  crypto: sun4i-ss: initialize need_fallback
  crypto: sun4i-ss: enabled stats via debugfs
  crypto: sun4i-ss: add SPDX header and remove blank lines

 drivers/crypto/allwinner/Kconfig              |  9 ++
 .../allwinner/sun4i-ss/sun4i-ss-cipher.c      | 87 +++++++++++++------
 .../crypto/allwinner/sun4i-ss/sun4i-ss-core.c | 56 ++++++++++++
 .../crypto/allwinner/sun4i-ss/sun4i-ss-hash.c |  6 ++
 .../crypto/allwinner/sun4i-ss/sun4i-ss-prng.c |  6 ++
 drivers/crypto/allwinner/sun4i-ss/sun4i-ss.h  |  8 ++
 6 files changed, 146 insertions(+), 26 deletions(-)

Comments

David Laight Nov. 16, 2020, 2:54 p.m. UTC | #1
From: Corentin Labbe
> Sent: 16 November 2020 13:54
> 
> The optimized cipher function need length multiple of 4 bytes.
> But it get sometimes odd length.
> This is due to SG data could be stored with an offset.
> 
> So the fix is to check also if the offset is aligned with 4 bytes.
> Fixes: 6298e948215f2 ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-
> ss/sun4i-ss-cipher.c
> index 19f1aa577ed4..4dd736ee5a4d 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> @@ -186,12 +186,12 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
>  	 * we can use the SS optimized function
>  	 */
>  	while (in_sg && no_chunk == 1) {
> -		if (in_sg->length % 4)
> +		if (in_sg->length % 4 || !IS_ALIGNED(in_sg->offset, sizeof(u32)))

You probably ought to do the test in a consistent manner.
Probably something that reduces to:
	((unsigned long)in_sg->offset | in_sg->length) & 3u

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Arnd Bergmann Nov. 16, 2020, 3:47 p.m. UTC | #2
On Mon, Nov 16, 2020 at 2:53 PM Corentin Labbe <clabbe@baylibre.com> wrote:
>
> Ciphers produce invalid results on BE.
> Key and IV need to be written in LE.
>
> Fixes: 6298e948215f2 ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
>  drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> index 53478c3feca6..8f4621826330 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> @@ -52,13 +52,13 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
>
>         spin_lock_irqsave(&ss->slock, flags);
>
> -       for (i = 0; i < op->keylen; i += 4)
> -               writel(*(op->key + i / 4), ss->base + SS_KEY0 + i);
> +       for (i = 0; i < op->keylen / 4; i++)
> +               writesl(ss->base + SS_KEY0 + i * 4, &op->key[i], 1);
>

This looks correct, but I wonder if we should just introduce
memcpy_toio32() and memcpy_fromio32() as a generic interface,
as this seems to come up occasionally, and the method here
(a loop around an inline function with another loop) is a bit clumsy.

      Arnd