* [PATCH v2 2/7] crypto: sun4i-ss: checking sg length is not sufficient
[not found] <1600627038-40000-1-git-send-email-clabbe@baylibre.com>
@ 2020-09-20 18:37 ` Corentin Labbe
2020-09-20 18:37 ` [PATCH v2 3/7] crypto: sun4i-ss: IV register does not work on A10 and A13 Corentin Labbe
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Corentin Labbe @ 2020-09-20 18:37 UTC (permalink / raw)
To: arnd, davem, herbert, mripard, wens
Cc: linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi,
Corentin Labbe, stable
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 b92d175b5d2a..2614640231dc 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
@@ -188,12 +188,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)))
no_chunk = 0;
in_sg = sg_next(in_sg);
}
while (out_sg && no_chunk == 1) {
- if (out_sg->length % 4)
+ if (out_sg->length % 4 || !IS_ALIGNED(out_sg->offset, sizeof(u32)))
no_chunk = 0;
out_sg = sg_next(out_sg);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 3/7] crypto: sun4i-ss: IV register does not work on A10 and A13
[not found] <1600627038-40000-1-git-send-email-clabbe@baylibre.com>
2020-09-20 18:37 ` [PATCH v2 2/7] crypto: sun4i-ss: checking sg length is not sufficient Corentin Labbe
@ 2020-09-20 18:37 ` Corentin Labbe
2020-09-20 18:37 ` [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher Corentin Labbe
2020-09-20 18:37 ` [PATCH v2 5/7] crypto: sun4i-ss: initialize need_fallback Corentin Labbe
3 siblings, 0 replies; 7+ messages in thread
From: Corentin Labbe @ 2020-09-20 18:37 UTC (permalink / raw)
To: arnd, davem, herbert, mripard, wens
Cc: linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi,
Corentin Labbe, stable
Allwinner A10 and A13 SoC have a version of the SS which produce
invalid IV in IVx register.
Instead of adding a variant for those, let's convert SS to produce IV
directly from data.
Fixes: 6298e948215f2 ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Cc: <stable@vger.kernel.org>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
.../allwinner/sun4i-ss/sun4i-ss-cipher.c | 34 +++++++++++++++----
1 file changed, 28 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 2614640231dc..c6c25204780d 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
@@ -20,6 +20,7 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
unsigned int ivsize = crypto_skcipher_ivsize(tfm);
struct sun4i_cipher_req_ctx *ctx = skcipher_request_ctx(areq);
u32 mode = ctx->mode;
+ void *backup_iv = NULL;
/* when activating SS, the default FIFO space is SS_RX_DEFAULT(32) */
u32 rx_cnt = SS_RX_DEFAULT;
u32 tx_cnt = 0;
@@ -42,6 +43,13 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
return -EINVAL;
}
+ if (areq->iv && ivsize > 0 && mode & SS_DECRYPTION) {
+ backup_iv = kzalloc(ivsize, GFP_KERNEL);
+ if (!backup_iv)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(backup_iv, areq->src, areq->cryptlen - ivsize, ivsize, 0);
+ }
+
spin_lock_irqsave(&ss->slock, flags);
for (i = 0; i < op->keylen; i += 4)
@@ -102,9 +110,12 @@ static int noinline_for_stack sun4i_ss_opti_poll(struct skcipher_request *areq)
} while (oleft);
if (areq->iv) {
- for (i = 0; i < 4 && i < ivsize / 4; i++) {
- v = readl(ss->base + SS_IV0 + i * 4);
- *(u32 *)(areq->iv + i * 4) = v;
+ if (mode & SS_DECRYPTION) {
+ memcpy(areq->iv, backup_iv, ivsize);
+ kfree_sensitive(backup_iv);
+ } else {
+ scatterwalk_map_and_copy(areq->iv, areq->dst, areq->cryptlen - ivsize,
+ ivsize, 0);
}
}
@@ -161,6 +172,7 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
unsigned int ileft = areq->cryptlen;
unsigned int oleft = areq->cryptlen;
unsigned int todo;
+ void *backup_iv = NULL;
struct sg_mapping_iter mi, mo;
unsigned int oi, oo; /* offset for in and out */
char buf[4 * SS_RX_MAX];/* buffer for linearize SG src */
@@ -204,6 +216,13 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
if (need_fallback)
return sun4i_ss_cipher_poll_fallback(areq);
+ if (areq->iv && ivsize > 0 && mode & SS_DECRYPTION) {
+ backup_iv = kzalloc(ivsize, GFP_KERNEL);
+ if (!backup_iv)
+ return -ENOMEM;
+ scatterwalk_map_and_copy(backup_iv, areq->src, areq->cryptlen - ivsize, ivsize, 0);
+ }
+
spin_lock_irqsave(&ss->slock, flags);
for (i = 0; i < op->keylen; i += 4)
@@ -324,9 +343,12 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
}
}
if (areq->iv) {
- for (i = 0; i < 4 && i < ivsize / 4; i++) {
- v = readl(ss->base + SS_IV0 + i * 4);
- *(u32 *)(areq->iv + i * 4) = v;
+ if (mode & SS_DECRYPTION) {
+ memcpy(areq->iv, backup_iv, ivsize);
+ kfree_sensitive(backup_iv);
+ } else {
+ scatterwalk_map_and_copy(areq->iv, areq->dst, areq->cryptlen - ivsize,
+ ivsize, 0);
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher
[not found] <1600627038-40000-1-git-send-email-clabbe@baylibre.com>
2020-09-20 18:37 ` [PATCH v2 2/7] crypto: sun4i-ss: checking sg length is not sufficient Corentin Labbe
2020-09-20 18:37 ` [PATCH v2 3/7] crypto: sun4i-ss: IV register does not work on A10 and A13 Corentin Labbe
@ 2020-09-20 18:37 ` Corentin Labbe
2020-09-23 14:00 ` Arnd Bergmann
2020-09-20 18:37 ` [PATCH v2 5/7] crypto: sun4i-ss: initialize need_fallback Corentin Labbe
3 siblings, 1 reply; 7+ messages in thread
From: Corentin Labbe @ 2020-09-20 18:37 UTC (permalink / raw)
To: arnd, davem, herbert, mripard, wens
Cc: linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi,
Corentin Labbe, stable
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 c6c25204780d..a05889745097 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);
@@ -225,13 +225,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);
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher
2020-09-20 18:37 ` [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher Corentin Labbe
@ 2020-09-23 14:00 ` Arnd Bergmann
2020-09-23 18:06 ` LABBE Corentin
0 siblings, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2020-09-23 14:00 UTC (permalink / raw)
To: Corentin Labbe
Cc: David Miller, Herbert Xu, Maxime Ripard, Chen-Yu Tsai, Linux ARM,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-kernel@vger.kernel.org, linux-sunxi, # 3.4.x
On Sun, Sep 20, 2020 at 8:37 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 c6c25204780d..a05889745097 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);
I suspect what you actually want here is writesl() in place of the
loop. This skips the byteswap on big-endian, rather than swapping
each word twice.
The point is that this register seems to act as a FIFO for a byte-stream
rather than a 32-bit fixed-endian register.
Arnd
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher
2020-09-23 14:00 ` Arnd Bergmann
@ 2020-09-23 18:06 ` LABBE Corentin
2020-09-23 18:59 ` Arnd Bergmann
0 siblings, 1 reply; 7+ messages in thread
From: LABBE Corentin @ 2020-09-23 18:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: David Miller, Herbert Xu, Maxime Ripard, Chen-Yu Tsai, Linux ARM,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-kernel@vger.kernel.org, linux-sunxi, # 3.4.x
On Wed, Sep 23, 2020 at 04:00:32PM +0200, Arnd Bergmann wrote:
> On Sun, Sep 20, 2020 at 8:37 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 c6c25204780d..a05889745097 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);
>
> I suspect what you actually want here is writesl() in place of the
> loop. This skips the byteswap on big-endian, rather than swapping
> each word twice.
>
> The point is that this register seems to act as a FIFO for a byte-stream
> rather than a 32-bit fixed-endian register.
>
> Arnd
Thanks, using writesl() fixes the warning, but I need to keep the loop since the register is different each time.
Or does it is better to use directly __raw_writel() ?
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher
2020-09-23 18:06 ` LABBE Corentin
@ 2020-09-23 18:59 ` Arnd Bergmann
0 siblings, 0 replies; 7+ messages in thread
From: Arnd Bergmann @ 2020-09-23 18:59 UTC (permalink / raw)
To: LABBE Corentin
Cc: Herbert Xu, linux-sunxi, linux-kernel@vger.kernel.org,
Maxime Ripard, Chen-Yu Tsai,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, # 3.4.x,
David Miller, Linux ARM
On Wed, Sep 23, 2020 at 8:08 PM LABBE Corentin <clabbe@baylibre.com> wrote:
> On Wed, Sep 23, 2020 at 04:00:32PM +0200, Arnd Bergmann wrote:
> > On Sun, Sep 20, 2020 at 8:37 PM Corentin Labbe <clabbe@baylibre.com> wrote:
> > > diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
> > > index c6c25204780d..a05889745097 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);
> >
> > I suspect what you actually want here is writesl() in place of the
> > loop. This skips the byteswap on big-endian, rather than swapping
> > each word twice.
> >
> > The point is that this register seems to act as a FIFO for a byte-stream
> > rather than a 32-bit fixed-endian register.
>
> Thanks, using writesl() fixes the warning, but I need to keep the loop
> since the register is different each time.
Ah, I see. I thought we had an interface for that as well, but I can't
find it now. I see memcpy_toio32() in one driver, but that implementation
appears to be wrong here (and probably also wrong for the machine
it was meant for)
There is the regular memcpy_toio(), but on big-endian Arm that
turns into a per-byte copy, which might either not work on your
hardware or be too slow.
There is also __iowrite32_copy(), which is not what I had remembered
but does seem to do what you want here.
> Or does it is better to use directly __raw_writel() ?
__raw_writel() is not very portable, so I would avoid that in normal
device drivers even when you only run them on specific hardware.
Arnd
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 5/7] crypto: sun4i-ss: initialize need_fallback
[not found] <1600627038-40000-1-git-send-email-clabbe@baylibre.com>
` (2 preceding siblings ...)
2020-09-20 18:37 ` [PATCH v2 4/7] crypto: sun4i-ss: handle BigEndian for cipher Corentin Labbe
@ 2020-09-20 18:37 ` Corentin Labbe
3 siblings, 0 replies; 7+ messages in thread
From: Corentin Labbe @ 2020-09-20 18:37 UTC (permalink / raw)
To: arnd, davem, herbert, mripard, wens
Cc: linux-arm-kernel, linux-crypto, linux-kernel, linux-sunxi,
Corentin Labbe, stable
The need_fallback is never initialized and seem to be always true at runtime.
So all hardware operations are always bypassed.
Fixes: 0ae1f46c55f87 ("crypto: sun4i-ss - fallback when length is not multiple of blocksize")
Cc: <stable@vger.kernel.org>
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
index a05889745097..f3bdf465b02e 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-cipher.c
@@ -181,7 +181,7 @@ static int sun4i_ss_cipher_poll(struct skcipher_request *areq)
unsigned int obo = 0; /* offset in bufo*/
unsigned int obl = 0; /* length of data in bufo */
unsigned long flags;
- bool need_fallback;
+ bool need_fallback = false;
if (!areq->cryptlen)
return 0;
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread