diff mbox series

[4/7] crypto: sun4i-ss: handle BigEndian for cipher

Message ID 1600367758-28589-5-git-send-email-clabbe@baylibre.com
State New
Headers show
Series None | expand

Commit Message

Corentin Labbe Sept. 17, 2020, 6:35 p.m. UTC
Ciphers produce invalid results on BE.
Key and IV need to be written in LE.
Furthermore, the non-optimized function is too complicated to convert,
let's simply fallback on BE for the moment.

Fixes: 6298e948215f2 ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Cc: <stable@vger.kernel.org>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
 .../crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

Comments

Corentin Labbe Sept. 18, 2020, 8:06 a.m. UTC | #1
On Fri, Sep 18, 2020 at 05:31:28PM +1000, Herbert Xu wrote:
> On Thu, Sep 17, 2020 at 06:35:55PM +0000, Corentin Labbe wrote:

> > Ciphers produce invalid results on BE.

> > Key and IV need to be written in LE.

> > Furthermore, the non-optimized function is too complicated to convert,

> > let's simply fallback on BE for the moment.

> > 

> > Fixes: 6298e948215f2 ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")

> > Cc: <stable@vger.kernel.org>

> > Signed-off-by: Corentin Labbe <clabbe@baylibre.com>

> > ---

> >  .../crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 17 +++++++++++------

> >  1 file changed, 11 insertions(+), 6 deletions(-)

> 

> Does the BE failure get caught by the selftest?

> 


Yes, selftest found it.

> If so please just leave it enabled so that it can be fixed properly.


Not sure to leave it enabled is a good idea.
A least, leaving it failing probably will not annoy any user (according to my readings of #linux-sunxi, nobody use BE).

But I think only me will see it and since I already have this on my TODO list, I dont see any interest to leave it failing.
Furthermore, having a clean BE boot will permit to enable BE boots for thoses SoCs on kernelCI.

Regards
Herbert Xu Sept. 18, 2020, 8:09 a.m. UTC | #2
On Fri, Sep 18, 2020 at 10:06:58AM +0200, LABBE Corentin wrote:
>

> But I think only me will see it and since I already have this on my TODO list, I dont see any interest to leave it failing.

> Furthermore, having a clean BE boot will permit to enable BE boots for thoses SoCs on kernelCI.


I'll happily accept patches that fix the actual bug but not ones
just papering over it.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Corentin Labbe Sept. 19, 2020, 7:05 p.m. UTC | #3
On Fri, Sep 18, 2020 at 06:09:15PM +1000, Herbert Xu wrote:
> On Fri, Sep 18, 2020 at 10:06:58AM +0200, LABBE Corentin wrote:

> >

> > But I think only me will see it and since I already have this on my TODO list, I dont see any interest to leave it failing.

> > Furthermore, having a clean BE boot will permit to enable BE boots for thoses SoCs on kernelCI.

> 

> I'll happily accept patches that fix the actual bug but not ones

> just papering over it.

> 


I am sorry, you are right.
Furthermore, while respining to fix it, it seems that the current fix is enough.
I have rerun a clean rebuild and test on A10/A13/A20/A33 with BE and sun4i-ss is working fine.

I will sent a clean v2.

Regards
diff mbox series

Patch

diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
index c6c25204780d..d66bb9cf657c 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++)
+		writel(cpu_to_le32(op->key[i]), ss->base + SS_KEY0 + i * 4);
 
 	if (areq->iv) {
 		for (i = 0; i < 4 && i < ivsize / 4; i++) {
 			v = *(u32 *)(areq->iv + i * 4);
-			writel(v, ss->base + SS_IV0 + i * 4);
+			writel(cpu_to_le32(v), ss->base + SS_IV0 + i * 4);
 		}
 	}
 	writel(mode, ss->base + SS_CTL);
@@ -213,6 +213,11 @@  static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
 	if (no_chunk == 1 && !need_fallback)
 		return sun4i_ss_opti_poll(areq);
 
+/* The non aligned function does not work on BE. Probably due to buf/bufo handling.*/
+#ifdef CONFIG_CPU_BIG_ENDIAN
+	need_fallback = true;
+#endif
+
 	if (need_fallback)
 		return sun4i_ss_cipher_poll_fallback(areq);
 
@@ -225,13 +230,13 @@  static int sun4i_ss_cipher_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++)
+		writel(cpu_to_le32(op->key[i]), ss->base + SS_KEY0 + i * 4);
 
 	if (areq->iv) {
 		for (i = 0; i < 4 && i < ivsize / 4; i++) {
 			v = *(u32 *)(areq->iv + i * 4);
-			writel(v, ss->base + SS_IV0 + i * 4);
+			writel(cpu_to_le32(v), ss->base + SS_IV0 + i * 4);
 		}
 	}
 	writel(mode, ss->base + SS_CTL);