encrypt_done called from interrupt context on rk3288 crypto driver

[Emil Karlson]

Greetings

It seems to me that rk3288 crypto driver calls encrypt_done from
interrupt context which causes runtime tests to fail.

This regression is probably introduced with patch:
crypto: xts - Convert to skcipher by Herbert Xu 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/crypto/xts.c?h=v4.12-rc2=f1c131b45410a202eb45cc55980a7a9e4e4b4f40

[   18.912744] random: crng init done
[   26.085491] [ cut here ]
[   26.087764] WARNING: CPU: 0 PID: 176 at crypto/skcipher.c:431
skcipher_walk_first+0x5c/0x14c
[   26.090045] Modules linked in:
[   26.092284] CPU: 0 PID: 176 Comm: cryptomgr_test Not tainted 4.12.0-rc2 #2
[   26.094546] Hardware name: Rockchip (Device Tree)
[   26.096799] [<8022f9d4>] (unwind_backtrace) from [<8022aeec>]
(show_stack+0x20/0x24)
[   26.099069] [<8022aeec>] (show_stack) from [<8060fc14>]
(dump_stack+0x88/0xa4)
[   26.101339] [<8060fc14>] (dump_stack) from [<80244f2c>] (__warn+0xf0/0x110)
[   26.103598] [<80244f2c>] (__warn) from [<80244ff0>]
(warn_slowpath_null+0x30/0x38)
[   26.105819] [<80244ff0>] (warn_slowpath_null) from [<805c3264>]
(skcipher_walk_first+0x5c/0x14c)
[   26.108054] [<805c3264>] (skcipher_walk_first) from [<805c341c>]
(skcipher_walk_skcipher+0xc8/0xcc)
[   26.110310] [<805c341c>] (skcipher_walk_skcipher) from [<805c365c>]
(skcipher_walk_virt+0x2c/0x48)
[   26.112593] [<805c365c>] (skcipher_walk_virt) from [<805cece4>]
(post_crypt+0x44/0x1cc)
[   26.114898] [<805cece4>] (post_crypt) from [<805cf33c>]
(encrypt_done+0x5c/0x7c)
[   26.117204] [<805cf33c>] (encrypt_done) from [<80874d40>]
(rk_crypto_complete+0x28/0x2c)
[   26.119544] [<80874d40>] (rk_crypto_complete) from [<80875044>]
(rk_ablk_rx+0x80/0x128)
[   26.121899] [<80875044>] (rk_ablk_rx) from [<80874358>]
(rk_crypto_irq_handle+0x68/0x94)
[   26.124294] [<80874358>] (rk_crypto_irq_handle) from [<8029838c>]
(__handle_irq_event_percpu+0x54/0x218)
[   26.126714] [<8029838c>] (__handle_irq_event_percpu) from
[<80298588>] (handle_irq_event_percpu+0x38/0x8c)
[   26.129152] [<80298588>] (handle_irq_event_percpu) from
[<80298630>] (handle_irq_event+0x54/0x78)
[   26.131601] [<80298630>] (handle_irq_event) from [<8029c3a0>]
(handle_fasteoi_irq+0xbc/0x140)
[   26.134096] [<8029c3a0>] (handle_fasteoi_irq) from [<80297afc>]
(generic_handle_irq+0x28/0x38)
[   26.136575] [<80297afc>] (generic_handle_irq) from [<80297bbc>]
(__handle_domain_irq+0xb0/0xc4)
[   26.139029] [<80297bbc>] (__handle_domain_irq) from [<80201584>]
(gic_handle_irq+0x58/0x84)
[   26.141497] [<80201584>] (gic_handle_irq) from [<80a73e78>]
(__irq_svc+0x58/0x74)
[   26.143955] Exception stack(0x821bdaa0 to 0x821bdae8)
[   26.146386] daa0: ed13ab80  6cafb000 44fb ed13ab80
82106400 810c12c0 8204de40
[   26.148869] dac0: edbbc2c0 6cafb000 80a6d804 821bdb34 821bdb38
821bdaf0 80a6d804 8026c3e8
[   26.151359] dae0: 60080013 
[   26.153842] [<80a73e78>] (__irq_svc) from [<8026c3e8>]
(finish_task_switch+0xf4/0x2b4)
^[[?1;2c[   26.156355] [<8026c3e8>] (finish_task_switch) from
[<80a6d804>] (__schedule+0x5f4/0x974)
[   26.158899] [<80a6d804>] (__schedule) from [<80a6dc64>]
(preempt_schedule_common+0x20/0x30)
[   26.161452] [<80a6dc64>] (preempt_schedule_common) from
[<80a6dcb4>] (_cond_resched+0x40/0x48)
[   26.164031] [<80a6dcb4>] (_cond_resched) from [<80a6e8bc>]
(wait_for_common+0x38/0x1a8)
[   26.166656] [<80a6e8bc>] (wait_for_common) from [<80a6ea4c>]
(wait_for_completion+0x20/0x24)
[   26.169243] [<80a6ea4c>] (wait_for_completion) from [<805c6fa0>]
(__test_skcipher+0x314/0x8a8)
[   26.171784] [<805c6fa0>] (__test_skcipher) from [<805c99a0>]
(test_skcipher+0x38/0xc4)
[   26.174299] [<805c99a0>] (test_skcipher) from [<805c9ac0>]
(alg_test_skcipher+0x94/0xb0)
^[[?1;2c^[[?1;2c[   26.176809] [<805c9ac0>] (alg_test_skcipher) from
[<805ca740>] (alg_test+0x240/0x2e0)^[[?1;2c
[   26.179325] [<805ca740>] (alg_test) from [<805c6458>]
(cryptomgr_test+0x34/0x54)
[   26.181841] [<805c6458>] (cryptomgr_test) from [<802641c8>]
(kthread+0x154/0x16c)
[   26.184359] [<802641c8>] (kthread) from [<80226e08>]
(ret_from_fork+0x14/0x2c)
[   26.186866] ---[ end trace 3311d538cc9cc0a4 ]---
[   26.189429] alg: skcipher: encryption failed on test 1 for
xts(ecb-aes-rk): ret=35
[   26.192050] device-mapper: table: 253:0: crypt: Error allocating crypto tfm
[   26.194682] device-mapper: ioctl: error adding target to table

Best Regards
-Emil

Re: [PATCH net-next 0/4] kernel TLS

[David Miller]

From: Dave Watson 
Date: Wed, 24 May 2017 09:26:33 -0700

> This series adds support for kernel TLS encryption over TCP sockets.
> A standard TCP socket is converted to a TLS socket using a setsockopt.
> Only symmetric crypto is done in the kernel, as well as TLS record
> framing.  The handshake remains in userspace, and the negotiated
> cipher keys/iv are provided to the TCP socket.
> 
> We implemented support for this API in OpenSSL 1.1.0, the code is
> available at https://github.com/Mellanox/tls-openssl/tree/master
> 
> It should work with any TLS library with similar modifications,
> a test tool using gnutls is here: https://github.com/Mellanox/tls-af_ktls_tool
> 
> Changes from RFC V2:
> 
> * Generic ULP (upper layer protocol) framework instead of TLS specific
>   setsockopts
> * Dropped Mellanox hardware patches, will come as separate series.
>   Framework will work for both.

The value in this is seeing how it can work transparently behind
an existing userspace library providing TLS services.

So the reference to the gnutls and openssl implementations probably
belongs in the Documentation patch #4 too.

Thanks.

Re: [PATCH net-next 2/4] tcp: export do_tcp_sendpages and tcp_rate_check_app_limited functions

[David Miller]

From: Dave Watson 
Date: Wed, 24 May 2017 09:26:57 -0700

> Export do_tcp_sendpages and tcp_rate_check_app_limited, since tls will need to
> sendpages while the socket is already locked.
> 
> tcp_sendpage is exported, but requires the socket lock to not be held already.
> 
> Signed-off-by: Aviad Yehezkel 
> Signed-off-by: Ilya Lesokhin 
> Signed-off-by: Boris Pismenny 
> Signed-off-by: Dave Watson 
 ...
> +EXPORT_SYMBOL(do_tcp_sendpages);
 ...
> +EXPORT_SYMBOL(tcp_rate_check_app_limited);

These need to both be EXPORT_SYMBOL_GPL().

[PATCH 1/1] crypto:drbg- Fixes panic in wait_for_completion call.

[Harsh Jain]

Initialise ctr_completion variable before use.

Signed-off-by: Harsh Jain 
---
 crypto/drbg.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index fa749f4..f1db29d 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1840,6 +1840,7 @@ static int drbg_kcapi_init(struct crypto_tfm *tfm)
 struct drbg_state *drbg = crypto_tfm_ctx(tfm);
 
 mutex_init(>drbg_mutex);
+init_completion(>ctr_completion);
 
 return 0;
 }
-- 
1.7.10.1

[PATCH v4 12/14] crypto: caampkc - comply with crypto_akcipher_maxsize()

[Tudor Ambarus]

crypto_akcipher_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

Signed-off-by: Tudor Ambarus 
---
 drivers/crypto/caam/caampkc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index 57f399c..9c508ba 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -911,12 +911,11 @@ static int caam_rsa_set_priv_key(struct crypto_akcipher 
*tfm, const void *key,
return -ENOMEM;
 }
 
-static int caam_rsa_max_size(struct crypto_akcipher *tfm)
+static unsigned int caam_rsa_max_size(struct crypto_akcipher *tfm)
 {
struct caam_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
-   struct caam_rsa_key *key = >key;
 
-   return (key->n) ? key->n_sz : -EINVAL;
+   return ctx->key.n_sz;
 }
 
 /* Per session pkc's driver context creation function */
-- 
2.7.4

[PATCH v4 11/14] crypto: rsa - comply with crypto_akcipher_maxsize()

[Tudor Ambarus]

crypto_akcipher_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

Signed-off-by: Tudor Ambarus 
---
 crypto/rsa.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/crypto/rsa.c b/crypto/rsa.c
index 4c280b6..b067f3a 100644
--- a/crypto/rsa.c
+++ b/crypto/rsa.c
@@ -337,11 +337,11 @@ static int rsa_set_priv_key(struct crypto_akcipher *tfm, 
const void *key,
return -ENOMEM;
 }
 
-static int rsa_max_size(struct crypto_akcipher *tfm)
+static unsigned int rsa_max_size(struct crypto_akcipher *tfm)
 {
struct rsa_mpi_key *pkey = akcipher_tfm_ctx(tfm);
 
-   return pkey->n ? mpi_get_size(pkey->n) : -EINVAL;
+   return mpi_get_size(pkey->n);
 }
 
 static void rsa_exit_tfm(struct crypto_akcipher *tfm)
-- 
2.7.4

[PATCH v4 13/14] crypto: qat - comply with crypto_akcipher_maxsize()

[Tudor Ambarus]

crypto_akcipher_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

Signed-off-by: Tudor Ambarus 
---
 drivers/crypto/qat/qat_common/qat_asym_algs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c 
b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index 1d882a7..6f5dd68 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -1256,11 +1256,11 @@ static int qat_rsa_setprivkey(struct crypto_akcipher 
*tfm, const void *key,
return qat_rsa_setkey(tfm, key, keylen, true);
 }
 
-static int qat_rsa_max_size(struct crypto_akcipher *tfm)
+static unsigned int qat_rsa_max_size(struct crypto_akcipher *tfm)
 {
struct qat_rsa_ctx *ctx = akcipher_tfm_ctx(tfm);
 
-   return (ctx->n) ? ctx->key_sz : -EINVAL;
+   return ctx->key_sz;
 }
 
 static int qat_rsa_init_tfm(struct crypto_akcipher *tfm)
-- 
2.7.4

[PATCH v4 14/14] crypto: pkcs1pad - comply with crypto_akcipher_maxsize()

[Tudor Ambarus]

crypto_akcipher_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

crypto_akcipher_maxsize() now returns an unsigned int.
Remove the unnecessary check.

Signed-off-by: Tudor Ambarus 
---
 crypto/rsa-pkcs1pad.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 8baab43..044bcfa 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -120,9 +120,6 @@ static int pkcs1pad_set_pub_key(struct crypto_akcipher 
*tfm, const void *key,
 
/* Find out new modulus size from rsa implementation */
err = crypto_akcipher_maxsize(ctx->child);
-   if (err < 0)
-   return err;
-
if (err > PAGE_SIZE)
return -ENOTSUPP;
 
@@ -144,9 +141,6 @@ static int pkcs1pad_set_priv_key(struct crypto_akcipher 
*tfm, const void *key,
 
/* Find out new modulus size from rsa implementation */
err = crypto_akcipher_maxsize(ctx->child);
-   if (err < 0)
-   return err;
-
if (err > PAGE_SIZE)
return -ENOTSUPP;
 
@@ -154,7 +148,7 @@ static int pkcs1pad_set_priv_key(struct crypto_akcipher 
*tfm, const void *key,
return 0;
 }
 
-static int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
+static unsigned int pkcs1pad_get_max_size(struct crypto_akcipher *tfm)
 {
struct pkcs1pad_ctx *ctx = akcipher_tfm_ctx(tfm);
 
@@ -164,7 +158,7 @@ static int pkcs1pad_get_max_size(struct crypto_akcipher 
*tfm)
 * decrypt/verify.
 */
 
-   return ctx->key_size ?: -EINVAL;
+   return ctx->key_size;
 }
 
 static void pkcs1pad_sg_set_buf(struct scatterlist *sg, void *buf, size_t len,
-- 
2.7.4

[PATCH v4 01/14] crypto: kpp, (ec)dh - fix typos

[Tudor Ambarus]

While here, add missing argument description (ndigits).

Signed-off-by: Tudor Ambarus 
---
 crypto/dh.c   | 4 ++--
 crypto/dh_helper.c| 4 ++--
 crypto/ecc.h  | 8 +---
 crypto/ecdh.c | 4 ++--
 crypto/ecdh_helper.c  | 4 ++--
 include/crypto/dh.h   | 4 ++--
 include/crypto/ecdh.h | 4 ++--
 include/crypto/kpp.h  | 4 ++--
 8 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 87e3542..7cec0498 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -4,9 +4,9 @@
  * Authors: Salvatore Benedetto 
  *
  * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
+ * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
+ * 2 of the License, or (at your option) any later version.
  */
 
 #include 
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 02db76b..8ba8a3f 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -3,9 +3,9 @@
  * Authors: Salvatore Benedetto 
  *
  * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
+ * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
+ * 2 of the License, or (at your option) any later version.
  */
 #include 
 #include 
diff --git a/crypto/ecc.h b/crypto/ecc.h
index 663d598..37f4385 100644
--- a/crypto/ecc.h
+++ b/crypto/ecc.h
@@ -34,9 +34,9 @@
  * ecc_is_key_valid() - Validate a given ECDH private key
  *
  * @curve_id:  id representing the curve to use
- * @ndigits:   curve number of digits
+ * @ndigits:   curve's number of digits
  * @private_key:   private key to be used for the given curve
- * @private_key_len:   private key len
+ * @private_key_len:   private key length
  *
  * Returns 0 if the key is acceptable, a negative value otherwise
  */
@@ -47,9 +47,10 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
  * ecdh_make_pub_key() - Compute an ECC public key
  *
  * @curve_id:  id representing the curve to use
+ * @ndigits:   curve's number of digits
  * @private_key:   pregenerated private key for the given curve
  * @private_key_len:   length of private_key
- * @public_key:buffer for storing the public key generated
+ * @public_key:buffer for storing the generated public key
  * @public_key_len:length of the public_key buffer
  *
  * Returns 0 if the public key was generated successfully, a negative value
@@ -63,6 +64,7 @@ int ecdh_make_pub_key(const unsigned int curve_id, unsigned 
int ndigits,
  * crypto_ecdh_shared_secret() - Compute a shared secret
  *
  * @curve_id:  id representing the curve to use
+ * @ndigits:   curve's number of digits
  * @private_key:   private key of part A
  * @private_key_len:   length of private_key
  * @public_key:public key of counterpart B
diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index 63ca337..3623307 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -4,9 +4,9 @@
  * Authors: Salvator Benedetto 
  *
  * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
+ * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
+ * 2 of the License, or (at your option) any later version.
  */
 
 #include 
diff --git a/crypto/ecdh_helper.c b/crypto/ecdh_helper.c
index 3cd8a24..f05bea5 100644
--- a/crypto/ecdh_helper.c
+++ b/crypto/ecdh_helper.c
@@ -3,9 +3,9 @@
  * Authors: Salvatore Benedetto 
  *
  * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public Licence
+ * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version
- * 2 of the Licence, or (at your option) any later version.
+ * 2 of the License, or (at your option) any later version.
  */
 #include 
 #include 
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 6b424ad..f638998 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -73,9 +73,9 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const 
struct dh *params);
 /**
  * crypto_dh_decode_key() - decode a private key
  * @buf:   Buffer holding a packet key that should be decoded
- * @len:   Lenth of the packet private key buffer
+ * @len:   Length of the packet private key buffer
  * @params:

[PATCH v4 09/14] crypto: qat - comply with crypto_kpp_maxsize()

[Tudor Ambarus]

crypto_kpp_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

Signed-off-by: Tudor Ambarus 
---
 drivers/crypto/qat/qat_common/qat_asym_algs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/qat_asym_algs.c 
b/drivers/crypto/qat/qat_common/qat_asym_algs.c
index 2aab80b..1d882a7 100644
--- a/drivers/crypto/qat/qat_common/qat_asym_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_asym_algs.c
@@ -521,11 +521,11 @@ static int qat_dh_set_secret(struct crypto_kpp *tfm, 
const void *buf,
return 0;
 }
 
-static int qat_dh_max_size(struct crypto_kpp *tfm)
+static unsigned int qat_dh_max_size(struct crypto_kpp *tfm)
 {
struct qat_dh_ctx *ctx = kpp_tfm_ctx(tfm);
 
-   return ctx->p ? ctx->p_size : -EINVAL;
+   return ctx->p_size;
 }
 
 static int qat_dh_init_tfm(struct crypto_kpp *tfm)
-- 
2.7.4

[PATCH v4 08/14] crypto: ecdh - comply with crypto_kpp_maxsize()

[Tudor Ambarus]

crypto_kpp_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

nbytes has no sense now, remove it and directly return the maxsize.

Signed-off-by: Tudor Ambarus 
---
 crypto/ecdh.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index ed1464a..4aa0b0c 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -104,13 +104,12 @@ static int ecdh_compute_value(struct kpp_request *req)
return ret;
 }
 
-static int ecdh_max_size(struct crypto_kpp *tfm)
+static unsigned int ecdh_max_size(struct crypto_kpp *tfm)
 {
struct ecdh_ctx *ctx = ecdh_get_ctx(tfm);
-   int nbytes = ctx->ndigits << ECC_DIGITS_TO_BYTES_SHIFT;
 
-   /* Public key is made of two coordinates */
-   return 2 * nbytes;
+   /* Public key is made of two coordinates, add one to the left shift */
+   return ctx->ndigits << (ECC_DIGITS_TO_BYTES_SHIFT + 1);
 }
 
 static void no_exit_tfm(struct crypto_kpp *tfm)
-- 
2.7.4

[PATCH v4 05/14] crypto: dh - fix memleak in setkey

[Tudor Ambarus]

setkey can be called multiple times during the existence
of the transformation object. In case of multiple setkey calls,
the old key was not freed and we leaked memory.
Free the old MPI key if any.

Signed-off-by: Tudor Ambarus 
---
 crypto/dh.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/crypto/dh.c b/crypto/dh.c
index 7cec0498..e151f12 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -85,6 +85,9 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void 
*buf,
struct dh_ctx *ctx = dh_get_ctx(tfm);
struct dh params;
 
+   /* Free the old MPI key if any */
+   dh_free_ctx(ctx);
+
if (crypto_dh_decode_key(buf, len, ) < 0)
return -EINVAL;
 
-- 
2.7.4

[PATCH v4 02/14] crypto: ecc - remove unused function arguments

[Tudor Ambarus]

Signed-off-by: Tudor Ambarus 
---
 crypto/ecc.c  |  8 +++-
 crypto/ecc.h  | 13 +++--
 crypto/ecdh.c | 11 +--
 3 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 414c78a..69b4cc4 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -928,8 +928,7 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
 }
 
 int ecdh_make_pub_key(unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, unsigned int private_key_len,
- u8 *public_key, unsigned int public_key_len)
+ const u8 *private_key, u8 *public_key)
 {
int ret = 0;
struct ecc_point *pk;
@@ -967,9 +966,8 @@ int ecdh_make_pub_key(unsigned int curve_id, unsigned int 
ndigits,
 }
 
 int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
-  const u8 *private_key, unsigned int private_key_len,
-  const u8 *public_key, unsigned int public_key_len,
-  u8 *secret, unsigned int secret_len)
+ const u8 *private_key, const u8 *public_key,
+ u8 *secret)
 {
int ret = 0;
struct ecc_point *product, *pk;
diff --git a/crypto/ecc.h b/crypto/ecc.h
index 37f4385..1ca9bf7 100644
--- a/crypto/ecc.h
+++ b/crypto/ecc.h
@@ -49,16 +49,13 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
  * @curve_id:  id representing the curve to use
  * @ndigits:   curve's number of digits
  * @private_key:   pregenerated private key for the given curve
- * @private_key_len:   length of private_key
  * @public_key:buffer for storing the generated public key
- * @public_key_len:length of the public_key buffer
  *
  * Returns 0 if the public key was generated successfully, a negative value
  * if an error occurred.
  */
 int ecdh_make_pub_key(const unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, unsigned int private_key_len,
- u8 *public_key, unsigned int public_key_len);
+ const u8 *private_key, u8 *public_key);
 
 /**
  * crypto_ecdh_shared_secret() - Compute a shared secret
@@ -66,11 +63,8 @@ int ecdh_make_pub_key(const unsigned int curve_id, unsigned 
int ndigits,
  * @curve_id:  id representing the curve to use
  * @ndigits:   curve's number of digits
  * @private_key:   private key of part A
- * @private_key_len:   length of private_key
  * @public_key:public key of counterpart B
- * @public_key_len:length of public_key
  * @secret:buffer for storing the calculated shared secret
- * @secret_len:length of the secret buffer
  *
  * Note: It is recommended that you hash the result of 
crypto_ecdh_shared_secret
  * before using it for symmetric encryption or HMAC.
@@ -79,7 +73,6 @@ int ecdh_make_pub_key(const unsigned int curve_id, unsigned 
int ndigits,
  * if an error occurred.
  */
 int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
-  const u8 *private_key, unsigned int private_key_len,
-  const u8 *public_key, unsigned int public_key_len,
-  u8 *secret, unsigned int secret_len);
+ const u8 *private_key, const u8 *public_key,
+ u8 *secret);
 #endif
diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index 3623307..69c3951 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -81,16 +81,15 @@ static int ecdh_compute_value(struct kpp_request *req)
return -EINVAL;
 
ret = crypto_ecdh_shared_secret(ctx->curve_id, ctx->ndigits,
-(const u8 *)ctx->private_key, nbytes,
-(const u8 *)ctx->public_key, 2 * 
nbytes,
-(u8 *)ctx->shared_secret, nbytes);
+   (const u8 *)ctx->private_key,
+   (const u8 *)ctx->public_key,
+   (u8 *)ctx->shared_secret);
 
buf = ctx->shared_secret;
} else {
ret = ecdh_make_pub_key(ctx->curve_id, ctx->ndigits,
-   (const u8 *)ctx->private_key, nbytes,
-   (u8 *)ctx->public_key,
-   sizeof(ctx->public_key));
+   (const u8 *)ctx->private_key,
+   (u8 *)ctx->public_key);
buf = ctx->public_key;
/* Public part is a point thus it has both coordinates */
nbytes *= 2;
-- 
2.7.4

[PATCH v4 03/14] crypto: ecc - remove unnecessary casts

[Tudor Ambarus]

ecc software implementation works with chunks of u64 data. There were some
unnecessary casts to u8 and then back to u64 for the ecc keys. This patch
removes the unnecessary casts.

Signed-off-by: Tudor Ambarus 
---
 crypto/ecc.c  | 28 +---
 crypto/ecc.h  |  8 
 crypto/ecdh.c | 11 +--
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index 69b4cc4..e3a2b8f 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -904,7 +904,7 @@ static inline void ecc_swap_digits(const u64 *in, u64 *out,
 }
 
 int ecc_is_key_valid(unsigned int curve_id, unsigned int ndigits,
-const u8 *private_key, unsigned int private_key_len)
+const u64 *private_key, unsigned int private_key_len)
 {
int nbytes;
const struct ecc_curve *curve = ecc_get_curve(curve_id);
@@ -917,23 +917,22 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
if (private_key_len != nbytes)
return -EINVAL;
 
-   if (vli_is_zero((const u64 *)_key[0], ndigits))
+   if (vli_is_zero(private_key, ndigits))
return -EINVAL;
 
/* Make sure the private key is in the range [1, n-1]. */
-   if (vli_cmp(curve->n, (const u64 *)_key[0], ndigits) != 1)
+   if (vli_cmp(curve->n, private_key, ndigits) != 1)
return -EINVAL;
 
return 0;
 }
 
 int ecdh_make_pub_key(unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, u8 *public_key)
+ const u64 *private_key, u64 *public_key)
 {
int ret = 0;
struct ecc_point *pk;
u64 priv[ndigits];
-   unsigned int nbytes;
const struct ecc_curve *curve = ecc_get_curve(curve_id);
 
if (!private_key || !curve) {
@@ -941,7 +940,7 @@ int ecdh_make_pub_key(unsigned int curve_id, unsigned int 
ndigits,
goto out;
}
 
-   ecc_swap_digits((const u64 *)private_key, priv, ndigits);
+   ecc_swap_digits(private_key, priv, ndigits);
 
pk = ecc_alloc_point(ndigits);
if (!pk) {
@@ -955,9 +954,8 @@ int ecdh_make_pub_key(unsigned int curve_id, unsigned int 
ndigits,
goto err_free_point;
}
 
-   nbytes = ndigits << ECC_DIGITS_TO_BYTES_SHIFT;
-   ecc_swap_digits(pk->x, (u64 *)public_key, ndigits);
-   ecc_swap_digits(pk->y, (u64 *)_key[nbytes], ndigits);
+   ecc_swap_digits(pk->x, public_key, ndigits);
+   ecc_swap_digits(pk->y, _key[ndigits], ndigits);
 
 err_free_point:
ecc_free_point(pk);
@@ -966,8 +964,8 @@ int ecdh_make_pub_key(unsigned int curve_id, unsigned int 
ndigits,
 }
 
 int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, const u8 *public_key,
- u8 *secret)
+ const u64 *private_key, const u64 *public_key,
+ u64 *secret)
 {
int ret = 0;
struct ecc_point *product, *pk;
@@ -997,13 +995,13 @@ int crypto_ecdh_shared_secret(unsigned int curve_id, 
unsigned int ndigits,
goto err_alloc_product;
}
 
-   ecc_swap_digits((const u64 *)public_key, pk->x, ndigits);
-   ecc_swap_digits((const u64 *)_key[nbytes], pk->y, ndigits);
-   ecc_swap_digits((const u64 *)private_key, priv, ndigits);
+   ecc_swap_digits(public_key, pk->x, ndigits);
+   ecc_swap_digits(_key[ndigits], pk->y, ndigits);
+   ecc_swap_digits(private_key, priv, ndigits);
 
ecc_point_mult(product, pk, priv, rand_z, curve->p, ndigits);
 
-   ecc_swap_digits(product->x, (u64 *)secret, ndigits);
+   ecc_swap_digits(product->x, secret, ndigits);
 
if (ecc_point_is_zero(product))
ret = -EFAULT;
diff --git a/crypto/ecc.h b/crypto/ecc.h
index 1ca9bf7..af2ffdb 100644
--- a/crypto/ecc.h
+++ b/crypto/ecc.h
@@ -41,7 +41,7 @@
  * Returns 0 if the key is acceptable, a negative value otherwise
  */
 int ecc_is_key_valid(unsigned int curve_id, unsigned int ndigits,
-const u8 *private_key, unsigned int private_key_len);
+const u64 *private_key, unsigned int private_key_len);
 
 /**
  * ecdh_make_pub_key() - Compute an ECC public key
@@ -55,7 +55,7 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
  * if an error occurred.
  */
 int ecdh_make_pub_key(const unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, u8 *public_key);
+ const u64 *private_key, u64 *public_key);
 
 /**
  * crypto_ecdh_shared_secret() - Compute a shared secret
@@ -73,6 +73,6 @@ int ecdh_make_pub_key(const unsigned int curve_id, unsigned 
int ndigits,
  * if an error occurred.
  */
 int crypto_ecdh_shared_secret(unsigned int curve_id, unsigned int ndigits,
- const u8 *private_key, const u8 

[PATCH v4 04/14] crypto: ecc - don't be selfish on pubkeys

[Tudor Ambarus]

Rename ecdh_make_pub_key() to ecc_make_pub_key().
This function might as well be used by ecdsa.

Signed-off-by: Tudor Ambarus 
---
 crypto/ecc.c  | 4 ++--
 crypto/ecc.h  | 4 ++--
 crypto/ecdh.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/crypto/ecc.c b/crypto/ecc.c
index e3a2b8f..6c33c43 100644
--- a/crypto/ecc.c
+++ b/crypto/ecc.c
@@ -927,8 +927,8 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
return 0;
 }
 
-int ecdh_make_pub_key(unsigned int curve_id, unsigned int ndigits,
- const u64 *private_key, u64 *public_key)
+int ecc_make_pub_key(unsigned int curve_id, unsigned int ndigits,
+const u64 *private_key, u64 *public_key)
 {
int ret = 0;
struct ecc_point *pk;
diff --git a/crypto/ecc.h b/crypto/ecc.h
index af2ffdb..673c834 100644
--- a/crypto/ecc.h
+++ b/crypto/ecc.h
@@ -54,8 +54,8 @@ int ecc_is_key_valid(unsigned int curve_id, unsigned int 
ndigits,
  * Returns 0 if the public key was generated successfully, a negative value
  * if an error occurred.
  */
-int ecdh_make_pub_key(const unsigned int curve_id, unsigned int ndigits,
- const u64 *private_key, u64 *public_key);
+int ecc_make_pub_key(const unsigned int curve_id, unsigned int ndigits,
+const u64 *private_key, u64 *public_key);
 
 /**
  * crypto_ecdh_shared_secret() - Compute a shared secret
diff --git a/crypto/ecdh.c b/crypto/ecdh.c
index c1f0163..ed1464a 100644
--- a/crypto/ecdh.c
+++ b/crypto/ecdh.c
@@ -87,8 +87,8 @@ static int ecdh_compute_value(struct kpp_request *req)
 
buf = ctx->shared_secret;
} else {
-   ret = ecdh_make_pub_key(ctx->curve_id, ctx->ndigits,
-   ctx->private_key, ctx->public_key);
+   ret = ecc_make_pub_key(ctx->curve_id, ctx->ndigits,
+  ctx->private_key, ctx->public_key);
buf = ctx->public_key;
/* Public part is a point thus it has both coordinates */
nbytes *= 2;
-- 
2.7.4

[PATCH v4 06/14] crypto: kpp: maxsize() - assume key is already set

[Tudor Ambarus]

As of now, crypto_kpp_maxsize() can not be reached without successfully
setting the key for the transformation. kpp algorithm implementations
check if the key was set and then return the output buffer size
required for the given key.

Change the return type to unsigned int and always assume that this
function is called after a successful setkey of the transformation.
kpp algorithm implementations will remove the check if key is not NULL
and directly return the max size.

Signed-off-by: Tudor Ambarus 
---
 include/crypto/kpp.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/crypto/kpp.h b/include/crypto/kpp.h
index c190825..2133d17 100644
--- a/include/crypto/kpp.h
+++ b/include/crypto/kpp.h
@@ -79,7 +79,7 @@ struct kpp_alg {
int (*generate_public_key)(struct kpp_request *req);
int (*compute_shared_secret)(struct kpp_request *req);
 
-   int (*max_size)(struct crypto_kpp *tfm);
+   unsigned int (*max_size)(struct crypto_kpp *tfm);
 
int (*init)(struct crypto_kpp *tfm);
void (*exit)(struct crypto_kpp *tfm);
@@ -323,13 +323,14 @@ static inline int crypto_kpp_compute_shared_secret(struct 
kpp_request *req)
 /**
  * crypto_kpp_maxsize() - Get len for output buffer
  *
- * Function returns the output buffer size required
+ * Function returns the output buffer size required for a given key.
+ * Function assumes that the key is already set in the transformation. If this
+ * function is called without a setkey or with a failed setkey, you will end up
+ * in a NULL dereference.
  *
  * @tfm:   KPP tfm handle allocated with crypto_alloc_kpp()
- *
- * Return: minimum len for output buffer or error code if key hasn't been set
  */
-static inline int crypto_kpp_maxsize(struct crypto_kpp *tfm)
+static inline unsigned int crypto_kpp_maxsize(struct crypto_kpp *tfm)
 {
struct kpp_alg *alg = crypto_kpp_alg(tfm);
 
-- 
2.7.4

[PATCH v4 07/14] crypto: dh - comply with crypto_kpp_maxsize()

[Tudor Ambarus]

crypto_kpp_maxsize() asks for the output buffer size without
caring for errors. It allways assume that will be called after
a valid setkey. Comply with it and return what he wants.

Signed-off-by: Tudor Ambarus 
---
 crypto/dh.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index e151f12..b1032a5 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -147,7 +147,7 @@ static int dh_compute_value(struct kpp_request *req)
return ret;
 }
 
-static int dh_max_size(struct crypto_kpp *tfm)
+static unsigned int dh_max_size(struct crypto_kpp *tfm)
 {
struct dh_ctx *ctx = dh_get_ctx(tfm);
 
-- 
2.7.4

[PATCH v4 10/14] crypto: akcipher: maxsize() - assume key is already set

[Tudor Ambarus]

As of now, crypto_akcipher_maxsize() can not be reached without
successfully setting the key for the transformation. akcipher
algorithm implementations check if the key was set and then return
the output buffer size required for the given key.

Change the return type to unsigned int and always assume that this
function is called after a successful setkey of the transformation.
akcipher algorithm implementations will remove the check if key is not NULL
and directly return the max size.

Signed-off-by: Tudor Ambarus 
---
 include/crypto/akcipher.h | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index c37cc59..b5e11de 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -98,7 +98,7 @@ struct akcipher_alg {
   unsigned int keylen);
int (*set_priv_key)(struct crypto_akcipher *tfm, const void *key,
unsigned int keylen);
-   int (*max_size)(struct crypto_akcipher *tfm);
+   unsigned int (*max_size)(struct crypto_akcipher *tfm);
int (*init)(struct crypto_akcipher *tfm);
void (*exit)(struct crypto_akcipher *tfm);
 
@@ -257,13 +257,14 @@ static inline void akcipher_request_set_crypt(struct 
akcipher_request *req,
 /**
  * crypto_akcipher_maxsize() - Get len for output buffer
  *
- * Function returns the dest buffer size required for a given key
+ * Function returns the dest buffer size required for a given key.
+ * Function assumes that the key is already set in the transformation. If this
+ * function is called without a setkey or with a failed setkey, you will end up
+ * in a NULL dereference.
  *
  * @tfm:   AKCIPHER tfm handle allocated with crypto_alloc_akcipher()
- *
- * Return: minimum len for output buffer or error code in key hasn't been set
  */
-static inline int crypto_akcipher_maxsize(struct crypto_akcipher *tfm)
+static inline unsigned int crypto_akcipher_maxsize(struct crypto_akcipher *tfm)
 {
struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
 
-- 
2.7.4

[PATCH v4 00/14] fixes for kpp and akcipher

[Tudor Ambarus]

Hi,

These are various fixes that I made while reading kpp and akcipher
implementations.

Changes in v4:
 - assume key is already set when calling crypto_akcipher/kpp_maxsize()

v3 can be found at:
http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg25312.html

Tudor Ambarus (14):
  crypto: kpp, (ec)dh - fix typos
  crypto: ecc - remove unused function arguments
  crypto: ecc - remove unnecessary casts
  crypto: ecc - don't be selfish on pubkeys
  crypto: dh - fix memleak in setkey
  crypto: kpp: maxsize() - assume key is already set
  crypto: dh - comply with crypto_kpp_maxsize()
  crypto: ecdh - comply with crypto_kpp_maxsize()
  crypto: qat - comply with crypto_kpp_maxsize()
  crypto: akcipher: maxsize() - assume key is already set
  crypto: rsa - comply with crypto_akcipher_maxsize()
  crypto: caampkc - comply with crypto_akcipher_maxsize()
  crypto: qat - comply with crypto_akcipher_maxsize()
  crypto: pkcs1pad - comply with crypto_akcipher_maxsize()

 crypto/dh.c   |  9 +---
 crypto/dh_helper.c|  4 ++--
 crypto/ecc.c  | 32 ---
 crypto/ecc.h  | 25 +
 crypto/ecdh.c | 25 +
 crypto/ecdh_helper.c  |  4 ++--
 crypto/rsa-pkcs1pad.c | 10 ++---
 crypto/rsa.c  |  4 ++--
 drivers/crypto/caam/caampkc.c |  5 ++---
 drivers/crypto/qat/qat_common/qat_asym_algs.c |  8 +++
 include/crypto/akcipher.h | 11 -
 include/crypto/dh.h   |  4 ++--
 include/crypto/ecdh.h |  4 ++--
 include/crypto/kpp.h  | 15 +++--
 14 files changed, 73 insertions(+), 87 deletions(-)

-- 
2.7.4