Re: 3.17 regression; alg: skcipher: Chunk test 1 failed on encryption at page 0 for ecb-aes-padlock

2014-10-20 Thread Ard Biesheuvel
On 20 October 2014 09:14, Jamie Heilman ja...@audible.transient.net wrote:
 I get this new failure w/3.17.0 on my system with a VIA Esther
 processor:

 alg: skcipher: Chunk test 1 failed on encryption at page 0 for ecb-aes-padlock
 : 71 73 f7 db 24 93 21 6d 61 1e bb 63 42 79 db 64
 0010: 6f 82 c0 ca a3 9b fa 0b d9 08 c7 4a 90 ae 8f 5f
 0020: 5e 06 f0 5f 31 51 18 37 45 d7 ca 3a fd 6c 3f e1
 0030: dd 8d 22 65 2b 00 50 ce ba 28 67 d7 ce 0e 0d ea
 0040: 78 69 7f ae 8f 8b 69 37 75 e0 dc 96 e0 b7 f4 09
 0050: cb 6d a2 fb da af 09 f8 81 82 27 fa 45 9c 29 a4
 0060: 22 8b 78 69 5b 46 f9 39 1b cc f9 1d 09 eb bc 5c
 0070: 41 72 51 97 1d 07 49 a0 1b 8e 65 4b b2 6a 12 03
 0080: 6a 60 95 ac bd ac 1a 64 de 5a a5 f0 83 2f cb ca
 0090: 22 74 a6 6c 9b 73 ce 3f e1 8b 22 17 59 0c 47 89
 00a0: 33 a1 d6 47 03 19 4f a8 67 69 f0 5b f0 20 ad 06
 00b0: 27 81 92 d8 c5 ba 98 12 be 24 b5 2f 75 02 c2 ad
 00c0: 12 2f 07 32 ee 39 af 64 05 8f b3 d4 eb 1b 46 6e
 00d0: d9 21 f9 c4 b7 c9 45 68 b4 a1 74 9f 82 47 eb cc
 00e0: bd 0a 14 95 0f 8b a8 2f 4b 1b a7 bf 82 a6 43 0c
 00f0: b9 39 4a a8 10 6f 50 7b 25 fb 26 81 e0 2f f0 96
 0100: 8d 8b ac 92 0f f6 ed 64 63 29 4c 8e 18 13 c5 bf
 0110: fc a0 d9 bf 7c 3a 0e 29 6f d1 6c 6f a5 da bf b1
 0120: 30 ea 44 2d c3 8f 16 e1 66 fa a3 21 3e fc 13 ca
 0130: f0 f6 f0 59 bd 8f 38 50 31 cb 69 3f 96 15 d6 f5
 0140: ae ff f6 aa 41 85 4c 10 58 e3 f9 44 e6 28 da 9a
 0150: dc 6a 80 34 73 97 1b c5 ca 26 16 77 0e 60 ab 89
 0160: 0f 04 27 bd ce 3e 71 b4 a0 d7 22 7e db eb 24 70
 0170: 42 71 51 78 70 b3 e0 3d 84 8e 8d 7b d0 6d ea 92
 0180: 11 08 42 4f e5 ad 26 92 d2 00 ae a8 e3 4b 37 47
 0190: 22 c1 95 c1 63 7f cb 03 f3 e3 d7 9d 60 c7 bc ea
 01a0: 35 a2 fd 45 52 39 13 6f c1 53 f3 53 df 33 84 d7
 01b0: d2 c8 37 b0 75 e3 41 46 b3 c7 83 2e 8a bb a4 e5
 01c0: 7f 3c fd 8b eb ea 63 bd b7 46 e7 bf 09 9c 0d 0f
 01d0: 33 84 aa 1c 8d 29 b4 ac 4f ad e6 89

 I've bisected this to 3b9b8fe0ade1ee84ee4058261d2e39a1f283704b so ...
 perhaps intended in terms of uncovering problems.  Seems to have
 identified something in my case at any rate.

 Attached is my full 3.17.0 dmesg, kernel config, and /proc/crypto contents
 (the only difference between 3.16 and 3.17 for the latter being the
 selftest value for ecb-aes-padlock which used to be passed with 3.16
 and earlier.)

 Let me know if you need anything else.


Interesting. I don't have access to the hardware, but I found
something interesting in the driver related to the prefetch size
(ecb_fetch_bytes) of ECB versus CBC (Note that the CBC selftest
passes)
So perhaps this might solve the bug, could you please test it?

diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index 633ba945e153..2834f0b23713 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -351,7 +351,7 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
padlock_reset_key(ctx-cword.encrypt);

blkcipher_walk_init(walk, dst, src, nbytes);
-   err = blkcipher_walk_virt(desc, walk);
+   err = blkcipher_walk_virt_block(desc, walk, ecb_fetch_bytes);

ts_state = irq_ts_save();
while ((nbytes = walk.nbytes)) {
@@ -380,7 +380,7 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
padlock_reset_key(ctx-cword.decrypt);

blkcipher_walk_init(walk, dst, src, nbytes);
-   err = blkcipher_walk_virt(desc, walk);
+   err = blkcipher_walk_virt_block(desc, walk, ecb_fetch_bytes);

ts_state = irq_ts_save();
while ((nbytes = walk.nbytes)) {

It will basically instruct the crypto layer not to pass fewer than 2
blocks at a time until there is really no other way, i.e., until the
input is exhausted.

-- 
Ard.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [linux-sunxi] [PATCH v5 2/4] ARM: sunxi: dt: Add DT bindings documentation for SUNXI Security System

2014-10-20 Thread Koen Kooi

 Op 19 okt. 2014, om 16:16 heeft LABBE Corentin clabbe.montj...@gmail.com 
 het volgende geschreven:
 
 This patch adds documentation for Device-Tree bindings for the Security 
 System cryptographic accelerator driver.
 
 Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
 ---
 Documentation/devicetree/bindings/crypto/sunxi-ss.txt | 9 +
 1 file changed, 9 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/crypto/sunxi-ss.txt
 
 diff --git a/Documentation/devicetree/bindings/crypto/sunxi-ss.txt 
 b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
 new file mode 100644
 index 000..a566803
 --- /dev/null
 +++ b/Documentation/devicetree/bindings/crypto/sunxi-ss.txt
 @@ -0,0 +1,9 @@
 +* Allwinner Security System found on A20 SoC
 +
 +Required properties:
 +- compatible : Should be allwinner,sun7i-a20-crypto.
 +- reg: Should contain the Security System register location and length.
 +- interrupts: Should contain the IRQ line for the Security System.
 +- clocks : A phandle to the functional clock node of the Security System 
 module
 +- clock-names : Name of the functional clock, should be ahb and mod.

It would be nice to have a copy/paste ready example node in here, like Boris 
did in [PATCH v6 2/2] mtd: nand: add sunxi NFC dt bindings doc.

regards,

Koen--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 4/4] crypto: Add Allwinner Security System crypto accelerator

2014-10-20 Thread Vladimir Zapolskiy
Hello LABBE,

On 19.10.2014 17:16, LABBE Corentin wrote:
 Add support for the Security System included in Allwinner SoC A20.
 The Security System is a hardware cryptographic accelerator that support 
 AES/MD5/SHA1/DES/3DES/PRNG algorithms.
 
 Signed-off-by: LABBE Corentin clabbe.montj...@gmail.com
 ---
  drivers/crypto/Kconfig|  17 ++
  drivers/crypto/Makefile   |   1 +
  drivers/crypto/sunxi-ss/Makefile  |   2 +
  drivers/crypto/sunxi-ss/sunxi-ss-cipher.c | 489 
 ++
  drivers/crypto/sunxi-ss/sunxi-ss-core.c   | 318 +++
  drivers/crypto/sunxi-ss/sunxi-ss-hash.c   | 445 +++
  drivers/crypto/sunxi-ss/sunxi-ss.h| 193 
  7 files changed, 1465 insertions(+)
  create mode 100644 drivers/crypto/sunxi-ss/Makefile
  create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
  create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-core.c
  create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss-hash.c
  create mode 100644 drivers/crypto/sunxi-ss/sunxi-ss.h
 
 diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
 index 2fb0fdf..9ba9759 100644
 --- a/drivers/crypto/Kconfig
 +++ b/drivers/crypto/Kconfig
 @@ -436,4 +436,21 @@ config CRYPTO_DEV_QCE
 hardware. To compile this driver as a module, choose M here. The
 module will be called qcrypto.
  
 +config CRYPTO_DEV_SUNXI_SS
 + tristate Support for Allwinner Security System cryptographic 
 accelerator
 + depends on ARCH_SUNXI
 + select CRYPTO_MD5
 + select CRYPTO_SHA1
 + select CRYPTO_AES
 + select CRYPTO_DES
 + select CRYPTO_BLKCIPHER
 + help
 +   Some Allwinner SoC have a crypto accelerator named
 +   Security System. Select this if you want to use it.
 +   The Security System handle AES/DES/3DES ciphers in CBC mode
 +   and SHA1 and MD5 hash algorithms.
 +
 +   To compile this driver as a module, choose M here: the module
 +   will be called sunxi-ss.
 +
  endif # CRYPTO_HW
 diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
 index 3924f93..856545c 100644
 --- a/drivers/crypto/Makefile
 +++ b/drivers/crypto/Makefile
 @@ -25,3 +25,4 @@ obj-$(CONFIG_CRYPTO_DEV_TALITOS) += talitos.o
  obj-$(CONFIG_CRYPTO_DEV_UX500) += ux500/
  obj-$(CONFIG_CRYPTO_DEV_QAT) += qat/
  obj-$(CONFIG_CRYPTO_DEV_QCE) += qce/
 +obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss/
 diff --git a/drivers/crypto/sunxi-ss/Makefile 
 b/drivers/crypto/sunxi-ss/Makefile
 new file mode 100644
 index 000..8bb287d
 --- /dev/null
 +++ b/drivers/crypto/sunxi-ss/Makefile
 @@ -0,0 +1,2 @@
 +obj-$(CONFIG_CRYPTO_DEV_SUNXI_SS) += sunxi-ss.o
 +sunxi-ss-y += sunxi-ss-core.o sunxi-ss-hash.o sunxi-ss-cipher.o
 diff --git a/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c 
 b/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
 new file mode 100644
 index 000..8d0416e
 --- /dev/null
 +++ b/drivers/crypto/sunxi-ss/sunxi-ss-cipher.c
 @@ -0,0 +1,489 @@
 +/*
 + * sunxi-ss-cipher.c - hardware cryptographic accelerator for Allwinner A20 
 SoC
 + *
 + * Copyright (C) 2013-2014 Corentin LABBE clabbe.montj...@gmail.com
 + *
 + * This file add support for AES cipher with 128,192,256 bits
 + * keysize in CBC mode.
 + * Add support also for DES and 3DES in CBC mode.
 + *
 + * You could find the datasheet in Documentation/arm/sunxi/README
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +#include sunxi-ss.h
 +
 +extern struct sunxi_ss_ctx *ss;
 +
 +static int sunxi_ss_cipher(struct ablkcipher_request *areq, u32 mode)
 +{
 + struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(areq);
 + struct sunxi_tfm_ctx *op = crypto_ablkcipher_ctx(tfm);
 + const char *cipher_type;
 +
 + if (areq-nbytes == 0)
 + return 0;
 +
 + if (areq-info == NULL) {
 + dev_err(ss-dev, ERROR: Empty IV\n);
 + return -EINVAL;
 + }
 +
 + if (areq-src == NULL || areq-dst == NULL) {
 + dev_err(ss-dev, ERROR: Some SGs are NULL\n);
 + return -EINVAL;
 + }
 +
 + cipher_type = crypto_tfm_alg_name(crypto_ablkcipher_tfm(tfm));
 +
 + if (strcmp(cbc(aes), cipher_type) == 0) {
 + mode |= SS_OP_AES | SS_CBC | SS_ENABLED | op-keymode;
 + return sunxi_ss_aes_poll(areq, mode);
 + }
 +
 + if (strcmp(cbc(des), cipher_type) == 0) {
 + mode |= SS_OP_DES | SS_CBC | SS_ENABLED | op-keymode;
 + return sunxi_ss_des_poll(areq, mode);
 + }
 +
 + if (strcmp(cbc(des3_ede), cipher_type) == 0) {
 + mode |= SS_OP_3DES | SS_CBC | SS_ENABLED | op-keymode;
 + return sunxi_ss_des_poll(areq, mode);
 + }
 +
 + dev_err(ss-dev, ERROR: Cipher %s not handled\n, cipher_type);
 + 

Re: 3.17 regression; alg: skcipher: Chunk test 1 failed on encryption at page 0 for ecb-aes-padlock

2014-10-20 Thread Jamie Heilman
Ard Biesheuvel wrote:
 On 20 October 2014 09:14, Jamie Heilman ja...@audible.transient.net wrote:
  I get this new failure w/3.17.0 on my system with a VIA Esther
  processor:
 
  alg: skcipher: Chunk test 1 failed on encryption at page 0 for 
  ecb-aes-padlock
  : 71 73 f7 db 24 93 21 6d 61 1e bb 63 42 79 db 64
  0010: 6f 82 c0 ca a3 9b fa 0b d9 08 c7 4a 90 ae 8f 5f
  0020: 5e 06 f0 5f 31 51 18 37 45 d7 ca 3a fd 6c 3f e1
  0030: dd 8d 22 65 2b 00 50 ce ba 28 67 d7 ce 0e 0d ea
  0040: 78 69 7f ae 8f 8b 69 37 75 e0 dc 96 e0 b7 f4 09
  0050: cb 6d a2 fb da af 09 f8 81 82 27 fa 45 9c 29 a4
  0060: 22 8b 78 69 5b 46 f9 39 1b cc f9 1d 09 eb bc 5c
  0070: 41 72 51 97 1d 07 49 a0 1b 8e 65 4b b2 6a 12 03
  0080: 6a 60 95 ac bd ac 1a 64 de 5a a5 f0 83 2f cb ca
  0090: 22 74 a6 6c 9b 73 ce 3f e1 8b 22 17 59 0c 47 89
  00a0: 33 a1 d6 47 03 19 4f a8 67 69 f0 5b f0 20 ad 06
  00b0: 27 81 92 d8 c5 ba 98 12 be 24 b5 2f 75 02 c2 ad
  00c0: 12 2f 07 32 ee 39 af 64 05 8f b3 d4 eb 1b 46 6e
  00d0: d9 21 f9 c4 b7 c9 45 68 b4 a1 74 9f 82 47 eb cc
  00e0: bd 0a 14 95 0f 8b a8 2f 4b 1b a7 bf 82 a6 43 0c
  00f0: b9 39 4a a8 10 6f 50 7b 25 fb 26 81 e0 2f f0 96
  0100: 8d 8b ac 92 0f f6 ed 64 63 29 4c 8e 18 13 c5 bf
  0110: fc a0 d9 bf 7c 3a 0e 29 6f d1 6c 6f a5 da bf b1
  0120: 30 ea 44 2d c3 8f 16 e1 66 fa a3 21 3e fc 13 ca
  0130: f0 f6 f0 59 bd 8f 38 50 31 cb 69 3f 96 15 d6 f5
  0140: ae ff f6 aa 41 85 4c 10 58 e3 f9 44 e6 28 da 9a
  0150: dc 6a 80 34 73 97 1b c5 ca 26 16 77 0e 60 ab 89
  0160: 0f 04 27 bd ce 3e 71 b4 a0 d7 22 7e db eb 24 70
  0170: 42 71 51 78 70 b3 e0 3d 84 8e 8d 7b d0 6d ea 92
  0180: 11 08 42 4f e5 ad 26 92 d2 00 ae a8 e3 4b 37 47
  0190: 22 c1 95 c1 63 7f cb 03 f3 e3 d7 9d 60 c7 bc ea
  01a0: 35 a2 fd 45 52 39 13 6f c1 53 f3 53 df 33 84 d7
  01b0: d2 c8 37 b0 75 e3 41 46 b3 c7 83 2e 8a bb a4 e5
  01c0: 7f 3c fd 8b eb ea 63 bd b7 46 e7 bf 09 9c 0d 0f
  01d0: 33 84 aa 1c 8d 29 b4 ac 4f ad e6 89
 
  I've bisected this to 3b9b8fe0ade1ee84ee4058261d2e39a1f283704b so ...
  perhaps intended in terms of uncovering problems.  Seems to have
  identified something in my case at any rate.
 
  Attached is my full 3.17.0 dmesg, kernel config, and /proc/crypto contents
  (the only difference between 3.16 and 3.17 for the latter being the
  selftest value for ecb-aes-padlock which used to be passed with 3.16
  and earlier.)
 
  Let me know if you need anything else.
 
 
 Interesting. I don't have access to the hardware, but I found
 something interesting in the driver related to the prefetch size
 (ecb_fetch_bytes) of ECB versus CBC (Note that the CBC selftest
 passes)
 So perhaps this might solve the bug, could you please test it?
 
 diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
 index 633ba945e153..2834f0b23713 100644
 --- a/drivers/crypto/padlock-aes.c
 +++ b/drivers/crypto/padlock-aes.c
 @@ -351,7 +351,7 @@ static int ecb_aes_encrypt(struct blkcipher_desc *desc,
 padlock_reset_key(ctx-cword.encrypt);
 
 blkcipher_walk_init(walk, dst, src, nbytes);
 -   err = blkcipher_walk_virt(desc, walk);
 +   err = blkcipher_walk_virt_block(desc, walk, ecb_fetch_bytes);
 
 ts_state = irq_ts_save();
 while ((nbytes = walk.nbytes)) {
 @@ -380,7 +380,7 @@ static int ecb_aes_decrypt(struct blkcipher_desc *desc,
 padlock_reset_key(ctx-cword.decrypt);
 
 blkcipher_walk_init(walk, dst, src, nbytes);
 -   err = blkcipher_walk_virt(desc, walk);
 +   err = blkcipher_walk_virt_block(desc, walk, ecb_fetch_bytes);
 
 ts_state = irq_ts_save();
 while ((nbytes = walk.nbytes)) {
 
 It will basically instruct the crypto layer not to pass fewer than 2
 blocks at a time until there is really no other way, i.e., until the
 input is exhausted.

Nope.  Test still fails w/exactly the same output as before.

-- 
Jamie Heilman http://audible.transient.net/~jamie/
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5 4/4] crypto: Add Allwinner Security System crypto accelerator

2014-10-20 Thread Joe Perches
On Tue, 2014-10-21 at 02:28 +0300, Vladimir Zapolskiy wrote:
 On 19.10.2014 17:16, LABBE Corentin wrote:
  Add support for the Security System included in Allwinner SoC A20.
  The Security System is a hardware cryptographic accelerator that support 
  AES/MD5/SHA1/DES/3DES/PRNG algorithms.
[]
  diff --git a/drivers/crypto/sunxi-ss/sunxi-ss-core.c 
  b/drivers/crypto/sunxi-ss/sunxi-ss-core.c
[]
  +   cr = clk_get_rate(ss-busclk);
  +   if (cr = cr_ahb)
  +   dev_dbg(pdev-dev, Clock bus %lu (%lu MHz) (must be = 
  %lu)\n,
  +   cr, cr / 100, cr_ahb);
  +   else
  +   dev_warn(pdev-dev, Clock bus %lu (%lu MHz) (must be = 
  %lu)\n,
  +   cr, cr / 100, cr_ahb);
 
 See next comment.
 
  +   cr = clk_get_rate(ss-ssclk);
  +   if (cr = cr_mod)
  +   if (cr  cr_mod)
  +   dev_info(pdev-dev, Clock ss %lu (%lu MHz) (must be 
  = %lu)\n,
  +   cr, cr / 100, cr_mod);
  +   else
  +   dev_dbg(pdev-dev, Clock ss %lu (%lu MHz) (must be = 
  %lu)\n,
  +   cr, cr / 100, cr_mod);
  +   else
  +   dev_warn(pdev-dev, Clock ss is at %lu (%lu MHz) (must be = 
  %lu)\n,
  +   cr, cr / 100, cr_mod);
 
 The management of kernel log levels looks pretty strange. As far as I
 understand there is no error on any clock rate, I'd recommend to keep
 only one information message.

And if not, please add some braces.

 hash_init: initialize request context */
  +int sunxi_hash_init(struct ahash_request *areq)
  +{
  +   const char *hash_type;
  +   struct sunxi_req_ctx *op = ahash_request_ctx(areq);
  +
  +   memset(op, 0, sizeof(struct sunxi_req_ctx));
  +
  +   hash_type = crypto_tfm_alg_name(areq-base.tfm);
  +
  +   if (strcmp(hash_type, sha1) == 0)
  +   op-mode = SS_OP_SHA1;
  +   if (strcmp(hash_type, md5) == 0)
  +   op-mode = SS_OP_MD5;

else if ?

  +   if (op-mode == 0)
  +   return -EINVAL;

maybe this?

if (!strcmp(hash_type, sha1))
op-mode = SS_OP_SHA1;
else if (!strcmp(hash_type, md5))
op-mode = SH_OP_MD5;
else
return -EINVAL;

  +
  +   return 0;
  +}
[]
  +int sunxi_hash_update(struct ahash_request *areq)
  +{
[]
  +   dev_dbg(ss-dev, %s %s bc=%llu len=%u mode=%x bw=%u ww=%u,
  +   __func__, crypto_tfm_alg_name(areq-base.tfm),
  +   op-byte_count, areq-nbytes, op-mode,
  +   op-nbw, op-nwait);

dev_dbg statements generally don't need __func__ as
dynamic_debug can add it.

If you want to keep it, the most common output form for
__func__ is '%s: ..., __func__'

--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html