linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] introduce crypto wait for async op
@ 2017-05-29  8:22 Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 01/11] crypto: " Gilad Ben-Yossef
                   ` (10 more replies)
  0 siblings, 11 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

Many users of kernel async. crypto services have a pattern of
starting an async. crypto op and than using a completion
to wait for it to end, resulting of the same code repeating
itself in multiple places, sometime with coding errors.

This patch aims to introduce a generic "wait for async.
crypto op to complete" functions and move all the users
I could find to use it.  This gets rid of almost 300
lines of code and uncovered a few bugs in existing code
(separate bug fix patch set sent previously).

The patch set was boot tested on x86_64 and arm64 which
at the very least tests the crypto users via testmgr
but I am less confident regarding some of the other
users and would love more testers.
 
Changes from v1:
- Address review comments from Eric Biggers.
- Separated out bug fixes of existing code and rebase
  on top of that patch set.
- Rename 'ecr' to 'wait' in fscrypto code.
- Split patch introducing the new API from the change
  moving over the algif code which it originated from
  to the new API.
- Inline crypto_wait_req().
- Some code indentation fixes.

Gilad Ben-Yossef (11):
  crypto: introduce crypto wait for async op
  crypto: move algif to generic async completion
  crypto: move pub key to generic async completion
  crypto: move drbg to generic async completion
  crypto: move gcm to generic async completion
  crypto: move testmgr to generic async completion
  dm: move dm-verity to generic async completion
  fscrypt: move to generic async completion
  cifs: move to generic async completion
  ima: move to generic async completion
  crypto: adapt api sample to use async. op wait

 Documentation/crypto/api-samples.rst |  52 ++--------
 crypto/af_alg.c                      |  27 -----
 crypto/algif_aead.c                  |  14 +--
 crypto/algif_hash.c                  |  29 +++---
 crypto/algif_skcipher.c              |  15 ++-
 crypto/api.c                         |  13 +++
 crypto/asymmetric_keys/public_key.c  |  28 +-----
 crypto/drbg.c                        |  34 ++-----
 crypto/gcm.c                         |  32 ++----
 crypto/testmgr.c                     | 184 +++++++++++------------------------
 drivers/md/dm-verity-target.c        |  81 ++++-----------
 drivers/md/dm-verity.h               |   5 -
 fs/cifs/smb2ops.c                    |  30 +-----
 fs/crypto/crypto.c                   |  28 +-----
 fs/crypto/fname.c                    |  36 ++-----
 fs/crypto/fscrypt_private.h          |  10 --
 fs/crypto/keyinfo.c                  |  21 +---
 include/crypto/drbg.h                |   3 +-
 include/crypto/if_alg.h              |  14 ---
 include/linux/crypto.h               |  41 ++++++++
 security/integrity/ima/ima_crypto.c  |  56 ++++-------
 21 files changed, 220 insertions(+), 533 deletions(-)

-- 
2.1.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 01/11] crypto: introduce crypto wait for async op
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-06-10  3:43   ` Herbert Xu
  2017-05-29  8:22 ` [PATCH v2 02/11] crypto: move algif to generic async completion Gilad Ben-Yossef
                   ` (9 subsequent siblings)
  10 siblings, 1 reply; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

Invoking a possibly async. crypto op and waiting for completion
while correctly handling backlog processing is a common task
in the crypto API implementation and outside users of it.

This patch adds a generic implementation for doing so in
preparation for using it across the board instead of hand
rolled versions.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: Eric Biggers <ebiggers3@gmail.com>
CC: Ofir Drang <ofir.drang@arm.com>
---
 crypto/api.c           | 13 +++++++++++++
 include/linux/crypto.h | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/crypto/api.c b/crypto/api.c
index 941cd4c..2a2479d 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -24,6 +24,7 @@
 #include <linux/sched/signal.h>
 #include <linux/slab.h>
 #include <linux/string.h>
+#include <linux/completion.h>
 #include "internal.h"
 
 LIST_HEAD(crypto_alg_list);
@@ -595,5 +596,17 @@ int crypto_has_alg(const char *name, u32 type, u32 mask)
 }
 EXPORT_SYMBOL_GPL(crypto_has_alg);
 
+void crypto_req_done(struct crypto_async_request *req, int err)
+{
+	struct crypto_wait *wait = req->data;
+
+	if (err == -EINPROGRESS)
+		return;
+
+	wait->err = err;
+	complete(&wait->completion);
+}
+EXPORT_SYMBOL_GPL(crypto_req_done);
+
 MODULE_DESCRIPTION("Cryptographic core API");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/crypto.h b/include/linux/crypto.h
index 84da997..bb00186 100644
--- a/include/linux/crypto.h
+++ b/include/linux/crypto.h
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
+#include <linux/completion.h>
 
 /*
  * Autoloaded crypto modules should only use a prefixed name to avoid allowing
@@ -468,6 +469,45 @@ struct crypto_alg {
 } CRYPTO_MINALIGN_ATTR;
 
 /*
+ * A helper struct for waiting for completion of async crypto ops
+ */
+struct crypto_wait {
+	struct completion completion;
+	int err;
+};
+
+/*
+ * Macro for declaring a crypto op async wait object on stack
+ */
+#define DECLARE_CRYPTO_WAIT(_wait) \
+	struct crypto_wait _wait = { \
+		COMPLETION_INITIALIZER_ONSTACK((_wait).completion), 0 }
+
+/*
+ * Async ops completion helper functioons
+ */
+void crypto_req_done(struct crypto_async_request *req, int err);
+
+static inline int crypto_wait_req(int err, struct crypto_wait *wait)
+{
+	switch (err) {
+	case -EINPROGRESS:
+	case -EBUSY:
+		wait_for_completion(&wait->completion);
+		reinit_completion(&wait->completion);
+		err = wait->err;
+		break;
+	};
+
+	return err;
+}
+
+static inline void crypto_init_wait(struct crypto_wait *wait)
+{
+	init_completion(&wait->completion);
+}
+
+/*
  * Algorithm registration interface.
  */
 int crypto_register_alg(struct crypto_alg *alg);
@@ -1604,5 +1644,6 @@ static inline int crypto_comp_decompress(struct crypto_comp *tfm,
 						    src, slen, dst, dlen);
 }
 
+
 #endif	/* _LINUX_CRYPTO_H */
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 02/11] crypto: move algif to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 01/11] crypto: " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 03/11] crypto: move pub key " Gilad Ben-Yossef
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

algif starts several async crypto ops and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/af_alg.c         | 27 ---------------------------
 crypto/algif_aead.c     | 14 +++++++-------
 crypto/algif_hash.c     | 29 +++++++++++++----------------
 crypto/algif_skcipher.c | 15 +++++++--------
 include/crypto/if_alg.h | 14 --------------
 5 files changed, 27 insertions(+), 72 deletions(-)

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 3556d8e..bf4acaf 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -480,33 +480,6 @@ int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con)
 }
 EXPORT_SYMBOL_GPL(af_alg_cmsg_send);
 
-int af_alg_wait_for_completion(int err, struct af_alg_completion *completion)
-{
-	switch (err) {
-	case -EINPROGRESS:
-	case -EBUSY:
-		wait_for_completion(&completion->completion);
-		reinit_completion(&completion->completion);
-		err = completion->err;
-		break;
-	};
-
-	return err;
-}
-EXPORT_SYMBOL_GPL(af_alg_wait_for_completion);
-
-void af_alg_complete(struct crypto_async_request *req, int err)
-{
-	struct af_alg_completion *completion = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	completion->err = err;
-	complete(&completion->completion);
-}
-EXPORT_SYMBOL_GPL(af_alg_complete);
-
 static int __init af_alg_init(void)
 {
 	int err = proto_register(&alg_proto, 0);
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 8af664f..677b336 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -57,7 +57,7 @@ struct aead_ctx {
 
 	void *iv;
 
-	struct af_alg_completion completion;
+	struct crypto_wait wait;
 
 	unsigned long used;
 
@@ -648,10 +648,10 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
 			       used, ctx->iv);
 	aead_request_set_ad(&ctx->aead_req, ctx->aead_assoclen);
 
-	err = af_alg_wait_for_completion(ctx->enc ?
-					 crypto_aead_encrypt(&ctx->aead_req) :
-					 crypto_aead_decrypt(&ctx->aead_req),
-					 &ctx->completion);
+	err = crypto_wait_req(ctx->enc ?
+			 crypto_aead_encrypt(&ctx->aead_req) :
+			 crypto_aead_decrypt(&ctx->aead_req),
+			 &ctx->wait);
 
 	if (err) {
 		/* EBADMSG implies a valid cipher operation took place */
@@ -912,7 +912,7 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk)
 	ctx->enc = 0;
 	ctx->tsgl.cur = 0;
 	ctx->aead_assoclen = 0;
-	af_alg_init_completion(&ctx->completion);
+	crypto_init_wait(&ctx->wait);
 	sg_init_table(ctx->tsgl.sg, ALG_MAX_PAGES);
 	INIT_LIST_HEAD(&ctx->list);
 
@@ -920,7 +920,7 @@ static int aead_accept_parent_nokey(void *private, struct sock *sk)
 
 	aead_request_set_tfm(&ctx->aead_req, aead);
 	aead_request_set_callback(&ctx->aead_req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  af_alg_complete, &ctx->completion);
+				  crypto_req_done, &ctx->wait);
 
 	sk->sk_destruct = aead_sock_destruct;
 
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 5e92bd2..a2fa6d1 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -26,7 +26,7 @@ struct hash_ctx {
 
 	u8 *result;
 
-	struct af_alg_completion completion;
+	struct crypto_wait wait;
 
 	unsigned int len;
 	bool more;
@@ -88,8 +88,7 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
 		if ((msg->msg_flags & MSG_MORE))
 			hash_free_result(sk, ctx);
 
-		err = af_alg_wait_for_completion(crypto_ahash_init(&ctx->req),
-						&ctx->completion);
+		err = crypto_wait_req(crypto_ahash_init(&ctx->req), &ctx->wait);
 		if (err)
 			goto unlock;
 	}
@@ -110,8 +109,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
 
 		ahash_request_set_crypt(&ctx->req, ctx->sgl.sg, NULL, len);
 
-		err = af_alg_wait_for_completion(crypto_ahash_update(&ctx->req),
-						 &ctx->completion);
+		err = crypto_wait_req(crypto_ahash_update(&ctx->req),
+				&ctx->wait);
 		af_alg_free_sg(&ctx->sgl);
 		if (err)
 			goto unlock;
@@ -129,8 +128,8 @@ static int hash_sendmsg(struct socket *sock, struct msghdr *msg,
 			goto unlock;
 
 		ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
-		err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
-						 &ctx->completion);
+		err = crypto_wait_req(crypto_ahash_final(&ctx->req),
+				 &ctx->wait);
 	}
 
 unlock:
@@ -171,7 +170,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
 	} else {
 		if (!ctx->more) {
 			err = crypto_ahash_init(&ctx->req);
-			err = af_alg_wait_for_completion(err, &ctx->completion);
+			err = crypto_wait_req(err, &ctx->wait);
 			if (err)
 				goto unlock;
 		}
@@ -179,7 +178,7 @@ static ssize_t hash_sendpage(struct socket *sock, struct page *page,
 		err = crypto_ahash_update(&ctx->req);
 	}
 
-	err = af_alg_wait_for_completion(err, &ctx->completion);
+	err = crypto_wait_req(err, &ctx->wait);
 	if (err)
 		goto unlock;
 
@@ -215,17 +214,15 @@ static int hash_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	ahash_request_set_crypt(&ctx->req, NULL, ctx->result, 0);
 
 	if (!result && !ctx->more) {
-		err = af_alg_wait_for_completion(
-				crypto_ahash_init(&ctx->req),
-				&ctx->completion);
+		err = crypto_wait_req(crypto_ahash_init(&ctx->req), &ctx->wait);
 		if (err)
 			goto unlock;
 	}
 
 	if (!result || ctx->more) {
 		ctx->more = 0;
-		err = af_alg_wait_for_completion(crypto_ahash_final(&ctx->req),
-						 &ctx->completion);
+		err = crypto_wait_req(crypto_ahash_final(&ctx->req),
+				&ctx->wait);
 		if (err)
 			goto unlock;
 	}
@@ -476,13 +473,13 @@ static int hash_accept_parent_nokey(void *private, struct sock *sk)
 	ctx->result = NULL;
 	ctx->len = len;
 	ctx->more = 0;
-	af_alg_init_completion(&ctx->completion);
+	crypto_init_wait(&ctx->wait);
 
 	ask->private = ctx;
 
 	ahash_request_set_tfm(&ctx->req, hash);
 	ahash_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   af_alg_complete, &ctx->completion);
+				   crypto_req_done, &ctx->wait);
 
 	sk->sk_destruct = hash_sock_destruct;
 
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 43839b0..75fa9a5 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -43,7 +43,7 @@ struct skcipher_ctx {
 
 	void *iv;
 
-	struct af_alg_completion completion;
+	struct crypto_wait wait;
 
 	atomic_t inflight;
 	size_t used;
@@ -684,11 +684,10 @@ static int skcipher_recvmsg_sync(struct socket *sock, struct msghdr *msg,
 		skcipher_request_set_crypt(&ctx->req, sg, ctx->rsgl.sg, used,
 					   ctx->iv);
 
-		err = af_alg_wait_for_completion(
-				ctx->enc ?
-					crypto_skcipher_encrypt(&ctx->req) :
-					crypto_skcipher_decrypt(&ctx->req),
-				&ctx->completion);
+		err = crypto_wait_req(ctx->enc ?
+				      crypto_skcipher_encrypt(&ctx->req) :
+				      crypto_skcipher_decrypt(&ctx->req),
+				      &ctx->wait);
 
 free:
 		af_alg_free_sg(&ctx->rsgl);
@@ -948,14 +947,14 @@ static int skcipher_accept_parent_nokey(void *private, struct sock *sk)
 	ctx->merge = 0;
 	ctx->enc = 0;
 	atomic_set(&ctx->inflight, 0);
-	af_alg_init_completion(&ctx->completion);
+	crypto_init_wait(&ctx->wait);
 
 	ask->private = ctx;
 
 	skcipher_request_set_tfm(&ctx->req, skcipher);
 	skcipher_request_set_callback(&ctx->req, CRYPTO_TFM_REQ_MAY_SLEEP |
 						 CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      af_alg_complete, &ctx->completion);
+				      crypto_req_done, &ctx->wait);
 
 	sk->sk_destruct = skcipher_sock_destruct;
 
diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index e2b9c6f..86764fb 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -14,7 +14,6 @@
 #define _CRYPTO_IF_ALG_H
 
 #include <linux/compiler.h>
-#include <linux/completion.h>
 #include <linux/if_alg.h>
 #include <linux/scatterlist.h>
 #include <linux/types.h>
@@ -37,11 +36,6 @@ struct alg_sock {
 	void *private;
 };
 
-struct af_alg_completion {
-	struct completion completion;
-	int err;
-};
-
 struct af_alg_control {
 	struct af_alg_iv *iv;
 	int op;
@@ -81,17 +75,9 @@ void af_alg_link_sg(struct af_alg_sgl *sgl_prev, struct af_alg_sgl *sgl_new);
 
 int af_alg_cmsg_send(struct msghdr *msg, struct af_alg_control *con);
 
-int af_alg_wait_for_completion(int err, struct af_alg_completion *completion);
-void af_alg_complete(struct crypto_async_request *req, int err);
-
 static inline struct alg_sock *alg_sk(struct sock *sk)
 {
 	return (struct alg_sock *)sk;
 }
 
-static inline void af_alg_init_completion(struct af_alg_completion *completion)
-{
-	init_completion(&completion->completion);
-}
-
 #endif	/* _CRYPTO_IF_ALG_H */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 03/11] crypto: move pub key to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 01/11] crypto: " Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 02/11] crypto: move algif to generic async completion Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 04/11] crypto: move drbg " Gilad Ben-Yossef
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

public_key_verify_signature() is starting an async crypto op and
waiting for it to complete. Move it over to generic code doing
the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/asymmetric_keys/public_key.c | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 3cd6e12..d916235 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -57,29 +57,13 @@ static void public_key_destroy(void *payload0, void *payload3)
 	public_key_signature_free(payload3);
 }
 
-struct public_key_completion {
-	struct completion completion;
-	int err;
-};
-
-static void public_key_verify_done(struct crypto_async_request *req, int err)
-{
-	struct public_key_completion *compl = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	compl->err = err;
-	complete(&compl->completion);
-}
-
 /*
  * Verify a signature using a public key.
  */
 int public_key_verify_signature(const struct public_key *pkey,
 				const struct public_key_signature *sig)
 {
-	struct public_key_completion compl;
+	struct crypto_wait cwait;
 	struct crypto_akcipher *tfm;
 	struct akcipher_request *req;
 	struct scatterlist sig_sg, digest_sg;
@@ -131,20 +115,16 @@ int public_key_verify_signature(const struct public_key *pkey,
 	sg_init_one(&digest_sg, output, outlen);
 	akcipher_request_set_crypt(req, &sig_sg, &digest_sg, sig->s_size,
 				   outlen);
-	init_completion(&compl.completion);
+	crypto_init_wait(&cwait);
 	akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				      CRYPTO_TFM_REQ_MAY_SLEEP,
-				      public_key_verify_done, &compl);
+				      crypto_req_done, &cwait);
 
 	/* Perform the verification calculation.  This doesn't actually do the
 	 * verification, but rather calculates the hash expected by the
 	 * signature and returns that to us.
 	 */
-	ret = crypto_akcipher_verify(req);
-	if ((ret == -EINPROGRESS) || (ret == -EBUSY)) {
-		wait_for_completion(&compl.completion);
-		ret = compl.err;
-	}
+	ret = crypto_wait_req(crypto_akcipher_verify(req), &cwait);
 	if (ret < 0)
 		goto out_free_output;
 
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 04/11] crypto: move drbg to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (2 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 03/11] crypto: move pub key " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 05/11] crypto: move gcm " Gilad Ben-Yossef
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

DRBG is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.

The code now also passes CRYPTO_TFM_REQ_MAY_SLEEP flag indicating
crypto request memory allocation may use GFP_KERNEL which should
be perfectly fine as the code is obviously sleeping for the
completion of the request any way.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/drbg.c         | 34 ++++++++--------------------------
 include/crypto/drbg.h |  3 +--
 2 files changed, 9 insertions(+), 28 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index cdb27ac..101d9f0 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1651,16 +1651,6 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg)
 	return 0;
 }
 
-static void drbg_skcipher_cb(struct crypto_async_request *req, int error)
-{
-	struct drbg_state *drbg = req->data;
-
-	if (error == -EINPROGRESS)
-		return;
-	drbg->ctr_async_err = error;
-	complete(&drbg->ctr_completion);
-}
-
 static int drbg_init_sym_kernel(struct drbg_state *drbg)
 {
 	struct crypto_cipher *tfm;
@@ -1699,8 +1689,9 @@ static int drbg_init_sym_kernel(struct drbg_state *drbg)
 		return -ENOMEM;
 	}
 	drbg->ctr_req = req;
-	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-					drbg_skcipher_cb, drbg);
+	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
+						CRYPTO_TFM_REQ_MAY_SLEEP,
+					crypto_req_done, &drbg->ctr_wait);
 
 	alignmask = crypto_skcipher_alignmask(sk_tfm);
 	drbg->ctr_null_value_buf = kzalloc(DRBG_CTR_NULL_LEN + alignmask,
@@ -1761,21 +1752,12 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 		/* Output buffer may not be valid for SGL, use scratchpad */
 		skcipher_request_set_crypt(drbg->ctr_req, &sg_in, &sg_out,
 					   cryptlen, drbg->V);
-		ret = crypto_skcipher_encrypt(drbg->ctr_req);
-		switch (ret) {
-		case 0:
-			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&drbg->ctr_completion);
-			if (!drbg->ctr_async_err) {
-				reinit_completion(&drbg->ctr_completion);
-				break;
-			}
-		default:
+		ret = crypto_wait_req(crypto_skcipher_encrypt(drbg->ctr_req),
+					&drbg->ctr_wait);
+		if (ret)
 			goto out;
-		}
-		init_completion(&drbg->ctr_completion);
+
+		crypto_init_wait(&drbg->ctr_wait);
 
 		memcpy(outbuf, drbg->outscratchpad, cryptlen);
 
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 22f884c..8f94110 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -126,8 +126,7 @@ struct drbg_state {
 	__u8 *ctr_null_value;			/* CTR mode aligned zero buf */
 	__u8 *outscratchpadbuf;			/* CTR mode output scratchpad */
         __u8 *outscratchpad;			/* CTR mode aligned outbuf */
-	struct completion ctr_completion;	/* CTR mode async handler */
-	int ctr_async_err;			/* CTR mode async error */
+	struct crypto_wait ctr_wait;		/* CTR mode async wait obj */
 
 	bool seeded;		/* DRBG fully seeded? */
 	bool pr;		/* Prediction resistance enabled? */
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 05/11] crypto: move gcm to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (3 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 04/11] crypto: move drbg " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 06/11] crypto: move testmgr " Gilad Ben-Yossef
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

gcm is starting an async. crypto op and waiting for it complete.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/gcm.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index 3841b5e..fb923a5 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -16,7 +16,6 @@
 #include <crypto/scatterwalk.h>
 #include <crypto/hash.h>
 #include "internal.h"
-#include <linux/completion.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -78,11 +77,6 @@ struct crypto_gcm_req_priv_ctx {
 	} u;
 };
 
-struct crypto_gcm_setkey_result {
-	int err;
-	struct completion completion;
-};
-
 static struct {
 	u8 buf[16];
 	struct scatterlist sg;
@@ -98,17 +92,6 @@ static inline struct crypto_gcm_req_priv_ctx *crypto_gcm_reqctx(
 	return (void *)PTR_ALIGN((u8 *)aead_request_ctx(req), align + 1);
 }
 
-static void crypto_gcm_setkey_done(struct crypto_async_request *req, int err)
-{
-	struct crypto_gcm_setkey_result *result = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	result->err = err;
-	complete(&result->completion);
-}
-
 static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 			     unsigned int keylen)
 {
@@ -119,7 +102,7 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 		be128 hash;
 		u8 iv[16];
 
-		struct crypto_gcm_setkey_result result;
+		struct crypto_wait wait;
 
 		struct scatterlist sg[1];
 		struct skcipher_request req;
@@ -140,21 +123,18 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 	if (!data)
 		return -ENOMEM;
 
-	init_completion(&data->result.completion);
+	crypto_init_wait(&data->wait);
 	sg_init_one(data->sg, &data->hash, sizeof(data->hash));
 	skcipher_request_set_tfm(&data->req, ctr);
 	skcipher_request_set_callback(&data->req, CRYPTO_TFM_REQ_MAY_SLEEP |
 						  CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      crypto_gcm_setkey_done,
-				      &data->result);
+				      crypto_req_done,
+				      &data->wait);
 	skcipher_request_set_crypt(&data->req, data->sg, data->sg,
 				   sizeof(data->hash), data->iv);
 
-	err = crypto_skcipher_encrypt(&data->req);
-	if (err == -EINPROGRESS || err == -EBUSY) {
-		wait_for_completion(&data->result.completion);
-		err = data->result.err;
-	}
+	err = crypto_wait_req(crypto_skcipher_encrypt(&data->req),
+							&data->wait);
 
 	if (err)
 		goto out;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 06/11] crypto: move testmgr to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (4 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 05/11] crypto: move gcm " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 07/11] dm: move dm-verity " Gilad Ben-Yossef
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

testmgr is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.

This also provides a test of the generic crypto async. wait code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/testmgr.c | 184 +++++++++++++++++--------------------------------------
 1 file changed, 56 insertions(+), 128 deletions(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 6f5f3ed..4b779ca 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -76,11 +76,6 @@ int alg_test(const char *driver, const char *alg, u32 type, u32 mask)
 #define ENCRYPT 1
 #define DECRYPT 0
 
-struct tcrypt_result {
-	struct completion completion;
-	int err;
-};
-
 struct aead_test_suite {
 	struct {
 		const struct aead_testvec *vecs;
@@ -155,17 +150,6 @@ static void hexdump(unsigned char *buf, unsigned int len)
 			buf, len, false);
 }
 
-static void tcrypt_complete(struct crypto_async_request *req, int err)
-{
-	struct tcrypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int testmgr_alloc_buf(char *buf[XBUFSIZE])
 {
 	int i;
@@ -193,20 +177,10 @@ static void testmgr_free_buf(char *buf[XBUFSIZE])
 		free_page((unsigned long)buf[i]);
 }
 
-static int wait_async_op(struct tcrypt_result *tr, int ret)
-{
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
-		wait_for_completion(&tr->completion);
-		reinit_completion(&tr->completion);
-		ret = tr->err;
-	}
-	return ret;
-}
-
 static int ahash_partial_update(struct ahash_request **preq,
 	struct crypto_ahash *tfm, const struct hash_testvec *template,
 	void *hash_buff, int k, int temp, struct scatterlist *sg,
-	const char *algo, char *result, struct tcrypt_result *tresult)
+	const char *algo, char *result, struct crypto_wait *wait)
 {
 	char *state;
 	struct ahash_request *req;
@@ -236,7 +210,7 @@ static int ahash_partial_update(struct ahash_request **preq,
 	}
 	ahash_request_set_callback(req,
 		CRYPTO_TFM_REQ_MAY_BACKLOG,
-		tcrypt_complete, tresult);
+		crypto_req_done, wait);
 
 	memcpy(hash_buff, template->plaintext + temp,
 		template->tap[k]);
@@ -247,7 +221,7 @@ static int ahash_partial_update(struct ahash_request **preq,
 		pr_err("alg: hash: Failed to import() for %s\n", algo);
 		goto out;
 	}
-	ret = wait_async_op(tresult, crypto_ahash_update(req));
+	ret = crypto_wait_req(crypto_ahash_update(req), wait);
 	if (ret)
 		goto out;
 	*preq = req;
@@ -272,7 +246,7 @@ static int __test_hash(struct crypto_ahash *tfm,
 	char *result;
 	char *key;
 	struct ahash_request *req;
-	struct tcrypt_result tresult;
+	struct crypto_wait wait;
 	void *hash_buff;
 	char *xbuf[XBUFSIZE];
 	int ret = -ENOMEM;
@@ -286,7 +260,7 @@ static int __test_hash(struct crypto_ahash *tfm,
 	if (testmgr_alloc_buf(xbuf))
 		goto out_nobuf;
 
-	init_completion(&tresult.completion);
+	crypto_init_wait(&wait);
 
 	req = ahash_request_alloc(tfm, GFP_KERNEL);
 	if (!req) {
@@ -295,7 +269,7 @@ static int __test_hash(struct crypto_ahash *tfm,
 		goto out_noreq;
 	}
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				   tcrypt_complete, &tresult);
+				   crypto_req_done, &wait);
 
 	j = 0;
 	for (i = 0; i < tcount; i++) {
@@ -335,26 +309,26 @@ static int __test_hash(struct crypto_ahash *tfm,
 
 		ahash_request_set_crypt(req, sg, result, template[i].psize);
 		if (use_digest) {
-			ret = wait_async_op(&tresult, crypto_ahash_digest(req));
+			ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
 			if (ret) {
 				pr_err("alg: hash: digest failed on test %d "
 				       "for %s: ret=%d\n", j, algo, -ret);
 				goto out;
 			}
 		} else {
-			ret = wait_async_op(&tresult, crypto_ahash_init(req));
+			ret = crypto_wait_req(crypto_ahash_init(req), &wait);
 			if (ret) {
 				pr_err("alt: hash: init failed on test %d "
 				       "for %s: ret=%d\n", j, algo, -ret);
 				goto out;
 			}
-			ret = wait_async_op(&tresult, crypto_ahash_update(req));
+			ret = crypto_wait_req(crypto_ahash_update(req), &wait);
 			if (ret) {
 				pr_err("alt: hash: update failed on test %d "
 				       "for %s: ret=%d\n", j, algo, -ret);
 				goto out;
 			}
-			ret = wait_async_op(&tresult, crypto_ahash_final(req));
+			ret = crypto_wait_req(crypto_ahash_final(req), &wait);
 			if (ret) {
 				pr_err("alt: hash: final failed on test %d "
 				       "for %s: ret=%d\n", j, algo, -ret);
@@ -420,22 +394,10 @@ static int __test_hash(struct crypto_ahash *tfm,
 		}
 
 		ahash_request_set_crypt(req, sg, result, template[i].psize);
-		ret = crypto_ahash_digest(req);
-		switch (ret) {
-		case 0:
-			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&tresult.completion);
-			reinit_completion(&tresult.completion);
-			ret = tresult.err;
-			if (!ret)
-				break;
-			/* fall through */
-		default:
-			printk(KERN_ERR "alg: hash: digest failed "
-			       "on chunking test %d for %s: "
-			       "ret=%d\n", j, algo, -ret);
+		ret = crypto_wait_req(crypto_ahash_digest(req), &wait);
+		if (ret) {
+			pr_err("alg: hash: digest failed on chunking test %d for %s: ret=%d\n",
+				j, algo, -ret);
 			goto out;
 		}
 
@@ -486,13 +448,13 @@ static int __test_hash(struct crypto_ahash *tfm,
 		}
 
 		ahash_request_set_crypt(req, sg, result, template[i].tap[0]);
-		ret = wait_async_op(&tresult, crypto_ahash_init(req));
+		ret = crypto_wait_req(crypto_ahash_init(req), &wait);
 		if (ret) {
 			pr_err("alt: hash: init failed on test %d for %s: ret=%d\n",
 				j, algo, -ret);
 			goto out;
 		}
-		ret = wait_async_op(&tresult, crypto_ahash_update(req));
+		ret = crypto_wait_req(crypto_ahash_update(req), &wait);
 		if (ret) {
 			pr_err("alt: hash: update failed on test %d for %s: ret=%d\n",
 				j, algo, -ret);
@@ -503,7 +465,7 @@ static int __test_hash(struct crypto_ahash *tfm,
 		for (k = 1; k < template[i].np; k++) {
 			ret = ahash_partial_update(&req, tfm, &template[i],
 				hash_buff, k, temp, &sg[0], algo, result,
-				&tresult);
+				&wait);
 			if (ret) {
 				pr_err("hash: partial update failed on test %d for %s: ret=%d\n",
 					j, algo, -ret);
@@ -511,7 +473,7 @@ static int __test_hash(struct crypto_ahash *tfm,
 			}
 			temp += template[i].tap[k];
 		}
-		ret = wait_async_op(&tresult, crypto_ahash_final(req));
+		ret = crypto_wait_req(crypto_ahash_final(req), &wait);
 		if (ret) {
 			pr_err("alt: hash: final failed on test %d for %s: ret=%d\n",
 				j, algo, -ret);
@@ -580,7 +542,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 	struct scatterlist *sg;
 	struct scatterlist *sgout;
 	const char *e, *d;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 	unsigned int authsize, iv_len;
 	void *input;
 	void *output;
@@ -619,7 +581,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 	else
 		e = "decryption";
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 
 	req = aead_request_alloc(tfm, GFP_KERNEL);
 	if (!req) {
@@ -629,7 +591,7 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 	}
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  tcrypt_complete, &result);
+				  crypto_req_done, &wait);
 
 	iv_len = crypto_aead_ivsize(tfm);
 
@@ -709,7 +671,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 
 		aead_request_set_ad(req, template[i].alen);
 
-		ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+		ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+					: crypto_aead_decrypt(req), &wait);
 
 		switch (ret) {
 		case 0:
@@ -722,13 +685,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 				goto out;
 			}
 			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&result.completion);
-			reinit_completion(&result.completion);
-			ret = result.err;
-			if (!ret)
-				break;
 		case -EBADMSG:
 			if (template[i].novrfy)
 				/* verification failure was expected */
@@ -866,7 +822,8 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 
 		aead_request_set_ad(req, template[i].alen);
 
-		ret = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
+		ret = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+					: crypto_aead_decrypt(req), &wait);
 
 		switch (ret) {
 		case 0:
@@ -879,13 +836,6 @@ static int __test_aead(struct crypto_aead *tfm, int enc,
 				goto out;
 			}
 			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&result.completion);
-			reinit_completion(&result.completion);
-			ret = result.err;
-			if (!ret)
-				break;
 		case -EBADMSG:
 			if (template[i].novrfy)
 				/* verification failure was expected */
@@ -1083,7 +1033,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 	struct scatterlist sg[8];
 	struct scatterlist sgout[8];
 	const char *e, *d;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 	void *data;
 	char iv[MAX_IVLEN];
 	char *xbuf[XBUFSIZE];
@@ -1107,7 +1057,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 	else
 		e = "decryption";
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 
 	req = skcipher_request_alloc(tfm, GFP_KERNEL);
 	if (!req) {
@@ -1117,7 +1067,7 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 	}
 
 	skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      tcrypt_complete, &result);
+				      crypto_req_done, &wait);
 
 	j = 0;
 	for (i = 0; i < tcount; i++) {
@@ -1164,23 +1114,12 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 
 		skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
 					   template[i].ilen, iv);
-		ret = enc ? crypto_skcipher_encrypt(req) :
-			    crypto_skcipher_decrypt(req);
+		ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+					crypto_skcipher_decrypt(req), &wait);
 
-		switch (ret) {
-		case 0:
-			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&result.completion);
-			reinit_completion(&result.completion);
-			ret = result.err;
-			if (!ret)
-				break;
-			/* fall through */
-		default:
+		if (ret) {
 			pr_err("alg: skcipher%s: %s failed on test %d for %s: ret=%d\n",
-			       d, e, j, algo, -ret);
+				d, e, j, algo, -ret);
 			goto out;
 		}
 
@@ -1272,21 +1211,10 @@ static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 		skcipher_request_set_crypt(req, sg, (diff_dst) ? sgout : sg,
 					   template[i].ilen, iv);
 
-		ret = enc ? crypto_skcipher_encrypt(req) :
-			    crypto_skcipher_decrypt(req);
+		ret = crypto_wait_req(enc ? crypto_skcipher_encrypt(req) :
+					crypto_skcipher_decrypt(req), &wait);
 
-		switch (ret) {
-		case 0:
-			break;
-		case -EINPROGRESS:
-		case -EBUSY:
-			wait_for_completion(&result.completion);
-			reinit_completion(&result.completion);
-			ret = result.err;
-			if (!ret)
-				break;
-			/* fall through */
-		default:
+		if (ret) {
 			pr_err("alg: skcipher%s: %s failed on chunk test %d for %s: ret=%d\n",
 			       d, e, j, algo, -ret);
 			goto out;
@@ -1462,7 +1390,7 @@ static int test_acomp(struct crypto_acomp *tfm,
 	int ret;
 	struct scatterlist src, dst;
 	struct acomp_req *req;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 
 	output = kmalloc(COMP_BUF_SIZE, GFP_KERNEL);
 	if (!output)
@@ -1486,7 +1414,7 @@ static int test_acomp(struct crypto_acomp *tfm,
 		}
 
 		memset(output, 0, dlen);
-		init_completion(&result.completion);
+		crypto_init_wait(&wait);
 		sg_init_one(&src, input_vec, ilen);
 		sg_init_one(&dst, output, dlen);
 
@@ -1501,9 +1429,9 @@ static int test_acomp(struct crypto_acomp *tfm,
 
 		acomp_request_set_params(req, &src, &dst, ilen, dlen);
 		acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-					   tcrypt_complete, &result);
+					   crypto_req_done, &wait);
 
-		ret = wait_async_op(&result, crypto_acomp_compress(req));
+		ret = crypto_wait_req(crypto_acomp_compress(req), &wait);
 		if (ret) {
 			pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
 			       i + 1, algo, -ret);
@@ -1516,10 +1444,10 @@ static int test_acomp(struct crypto_acomp *tfm,
 		dlen = COMP_BUF_SIZE;
 		sg_init_one(&src, output, ilen);
 		sg_init_one(&dst, decomp_out, dlen);
-		init_completion(&result.completion);
+		crypto_init_wait(&wait);
 		acomp_request_set_params(req, &src, &dst, ilen, dlen);
 
-		ret = wait_async_op(&result, crypto_acomp_decompress(req));
+		ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
 		if (ret) {
 			pr_err("alg: acomp: compression failed on test %d for %s: ret=%d\n",
 			       i + 1, algo, -ret);
@@ -1563,7 +1491,7 @@ static int test_acomp(struct crypto_acomp *tfm,
 		}
 
 		memset(output, 0, dlen);
-		init_completion(&result.completion);
+		crypto_init_wait(&wait);
 		sg_init_one(&src, input_vec, ilen);
 		sg_init_one(&dst, output, dlen);
 
@@ -1578,9 +1506,9 @@ static int test_acomp(struct crypto_acomp *tfm,
 
 		acomp_request_set_params(req, &src, &dst, ilen, dlen);
 		acomp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-					   tcrypt_complete, &result);
+					   crypto_req_done, &wait);
 
-		ret = wait_async_op(&result, crypto_acomp_decompress(req));
+		ret = crypto_wait_req(crypto_acomp_decompress(req), &wait);
 		if (ret) {
 			pr_err("alg: acomp: decompression failed on test %d for %s: ret=%d\n",
 			       i + 1, algo, -ret);
@@ -1997,7 +1925,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
 	struct kpp_request *req;
 	void *input_buf = NULL;
 	void *output_buf = NULL;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 	unsigned int out_len_max;
 	int err = -ENOMEM;
 	struct scatterlist src, dst;
@@ -2006,7 +1934,7 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
 	if (!req)
 		return err;
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 
 	err = crypto_kpp_set_secret(tfm, vec->secret, vec->secret_size);
 	if (err < 0)
@@ -2024,10 +1952,10 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
 	sg_init_one(&dst, output_buf, out_len_max);
 	kpp_request_set_output(req, &dst, out_len_max);
 	kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				 tcrypt_complete, &result);
+				 crypto_req_done, &wait);
 
 	/* Compute public key */
-	err = wait_async_op(&result, crypto_kpp_generate_public_key(req));
+	err = crypto_wait_req(crypto_kpp_generate_public_key(req), &wait);
 	if (err) {
 		pr_err("alg: %s: generate public key test failed. err %d\n",
 		       alg, err);
@@ -2055,8 +1983,8 @@ static int do_test_kpp(struct crypto_kpp *tfm, const struct kpp_testvec *vec,
 	kpp_request_set_input(req, &src, vec->b_public_size);
 	kpp_request_set_output(req, &dst, out_len_max);
 	kpp_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				 tcrypt_complete, &result);
-	err = wait_async_op(&result, crypto_kpp_compute_shared_secret(req));
+				 crypto_req_done, &wait);
+	err = crypto_wait_req(crypto_kpp_compute_shared_secret(req), &wait);
 	if (err) {
 		pr_err("alg: %s: compute shard secret test failed. err %d\n",
 		       alg, err);
@@ -2125,7 +2053,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
 	struct akcipher_request *req;
 	void *outbuf_enc = NULL;
 	void *outbuf_dec = NULL;
-	struct tcrypt_result result;
+	struct crypto_wait wait;
 	unsigned int out_len_max, out_len = 0;
 	int err = -ENOMEM;
 	struct scatterlist src, dst, src_tab[2];
@@ -2137,7 +2065,7 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
 	if (!req)
 		goto free_xbuf;
 
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 
 	if (vecs->public_key_vec)
 		err = crypto_akcipher_set_pub_key(tfm, vecs->key,
@@ -2166,10 +2094,10 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
 	akcipher_request_set_crypt(req, src_tab, &dst, vecs->m_size,
 				   out_len_max);
 	akcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				      tcrypt_complete, &result);
+				      crypto_req_done, &wait);
 
 	/* Run RSA encrypt - c = m^e mod n;*/
-	err = wait_async_op(&result, crypto_akcipher_encrypt(req));
+	err = crypto_wait_req(crypto_akcipher_encrypt(req), &wait);
 	if (err) {
 		pr_err("alg: akcipher: encrypt test failed. err %d\n", err);
 		goto free_all;
@@ -2204,11 +2132,11 @@ static int test_akcipher_one(struct crypto_akcipher *tfm,
 
 	sg_init_one(&src, xbuf[0], vecs->c_size);
 	sg_init_one(&dst, outbuf_dec, out_len_max);
-	init_completion(&result.completion);
+	crypto_init_wait(&wait);
 	akcipher_request_set_crypt(req, &src, &dst, vecs->c_size, out_len_max);
 
 	/* Run RSA decrypt - m = c^d mod n;*/
-	err = wait_async_op(&result, crypto_akcipher_decrypt(req));
+	err = crypto_wait_req(crypto_akcipher_decrypt(req), &wait);
 	if (err) {
 		pr_err("alg: akcipher: decrypt test failed. err %d\n", err);
 		goto free_all;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 07/11] dm: move dm-verity to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (5 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 06/11] crypto: move testmgr " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 08/11] fscrypt: move " Gilad Ben-Yossef
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

dm-verity is starting async. crypto ops and waiting for them to complete.
Move it over to generic code doing the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 drivers/md/dm-verity-target.c | 81 +++++++++++--------------------------------
 drivers/md/dm-verity.h        |  5 ---
 2 files changed, 20 insertions(+), 66 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 1ec9b2c..f183b43 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -92,74 +92,33 @@ static sector_t verity_position_at_level(struct dm_verity *v, sector_t block,
 	return block >> (level * v->hash_per_block_bits);
 }
 
-/*
- * Callback function for asynchrnous crypto API completion notification
- */
-static void verity_op_done(struct crypto_async_request *base, int err)
-{
-	struct verity_result *res = (struct verity_result *)base->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
-/*
- * Wait for async crypto API callback
- */
-static inline int verity_complete_op(struct verity_result *res, int ret)
-{
-	switch (ret) {
-	case 0:
-		break;
-
-	case -EINPROGRESS:
-	case -EBUSY:
-		ret = wait_for_completion_interruptible(&res->completion);
-		if (!ret)
-			ret = res->err;
-		reinit_completion(&res->completion);
-		break;
-
-	default:
-		DMERR("verity_wait_hash: crypto op submission failed: %d", ret);
-	}
-
-	if (unlikely(ret < 0))
-		DMERR("verity_wait_hash: crypto op failed: %d", ret);
-
-	return ret;
-}
-
 static int verity_hash_update(struct dm_verity *v, struct ahash_request *req,
 				const u8 *data, size_t len,
-				struct verity_result *res)
+				struct crypto_wait *wait)
 {
 	struct scatterlist sg;
 
 	sg_init_one(&sg, data, len);
 	ahash_request_set_crypt(req, &sg, NULL, len);
 
-	return verity_complete_op(res, crypto_ahash_update(req));
+	return crypto_wait_req(crypto_ahash_update(req), wait);
 }
 
 /*
  * Wrapper for crypto_ahash_init, which handles verity salting.
  */
 static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
-				struct verity_result *res)
+				struct crypto_wait *wait)
 {
 	int r;
 
 	ahash_request_set_tfm(req, v->tfm);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP |
 					CRYPTO_TFM_REQ_MAY_BACKLOG,
-					verity_op_done, (void *)res);
-	init_completion(&res->completion);
+					crypto_req_done, (void *)wait);
+	crypto_init_wait(wait);
 
-	r = verity_complete_op(res, crypto_ahash_init(req));
+	r = crypto_wait_req(crypto_ahash_init(req), wait);
 
 	if (unlikely(r < 0)) {
 		DMERR("crypto_ahash_init failed: %d", r);
@@ -167,18 +126,18 @@ static int verity_hash_init(struct dm_verity *v, struct ahash_request *req,
 	}
 
 	if (likely(v->salt_size && (v->version >= 1)))
-		r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+		r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
 	return r;
 }
 
 static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
-			     u8 *digest, struct verity_result *res)
+			     u8 *digest, struct crypto_wait *wait)
 {
 	int r;
 
 	if (unlikely(v->salt_size && (!v->version))) {
-		r = verity_hash_update(v, req, v->salt, v->salt_size, res);
+		r = verity_hash_update(v, req, v->salt, v->salt_size, wait);
 
 		if (r < 0) {
 			DMERR("verity_hash_final failed updating salt: %d", r);
@@ -187,7 +146,7 @@ static int verity_hash_final(struct dm_verity *v, struct ahash_request *req,
 	}
 
 	ahash_request_set_crypt(req, NULL, digest, 0);
-	r = verity_complete_op(res, crypto_ahash_final(req));
+	r = crypto_wait_req(crypto_ahash_final(req), wait);
 out:
 	return r;
 }
@@ -196,17 +155,17 @@ int verity_hash(struct dm_verity *v, struct ahash_request *req,
 		const u8 *data, size_t len, u8 *digest)
 {
 	int r;
-	struct verity_result res;
+	struct crypto_wait wait;
 
-	r = verity_hash_init(v, req, &res);
+	r = verity_hash_init(v, req, &wait);
 	if (unlikely(r < 0))
 		goto out;
 
-	r = verity_hash_update(v, req, data, len, &res);
+	r = verity_hash_update(v, req, data, len, &wait);
 	if (unlikely(r < 0))
 		goto out;
 
-	r = verity_hash_final(v, req, digest, &res);
+	r = verity_hash_final(v, req, digest, &wait);
 
 out:
 	return r;
@@ -389,7 +348,7 @@ int verity_hash_for_block(struct dm_verity *v, struct dm_verity_io *io,
  * Calculates the digest for the given bio
  */
 int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
-			struct bvec_iter *iter, struct verity_result *res)
+			struct bvec_iter *iter, struct crypto_wait *wait)
 {
 	unsigned int todo = 1 << v->data_dev_block_bits;
 	struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
@@ -414,7 +373,7 @@ int verity_for_io_block(struct dm_verity *v, struct dm_verity_io *io,
 		 */
 		sg_set_page(&sg, bv.bv_page, len, bv.bv_offset);
 		ahash_request_set_crypt(req, &sg, NULL, len);
-		r = verity_complete_op(res, crypto_ahash_update(req));
+		r = crypto_wait_req(crypto_ahash_update(req), wait);
 
 		if (unlikely(r < 0)) {
 			DMERR("verity_for_io_block crypto op failed: %d", r);
@@ -482,7 +441,7 @@ static int verity_verify_io(struct dm_verity_io *io)
 	struct dm_verity *v = io->v;
 	struct bvec_iter start;
 	unsigned b;
-	struct verity_result res;
+	struct crypto_wait wait;
 
 	for (b = 0; b < io->n_blocks; b++) {
 		int r;
@@ -507,17 +466,17 @@ static int verity_verify_io(struct dm_verity_io *io)
 			continue;
 		}
 
-		r = verity_hash_init(v, req, &res);
+		r = verity_hash_init(v, req, &wait);
 		if (unlikely(r < 0))
 			return r;
 
 		start = io->iter;
-		r = verity_for_io_block(v, io, &io->iter, &res);
+		r = verity_for_io_block(v, io, &io->iter, &wait);
 		if (unlikely(r < 0))
 			return r;
 
 		r = verity_hash_final(v, req, verity_io_real_digest(v, io),
-					&res);
+					&wait);
 		if (unlikely(r < 0))
 			return r;
 
diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index a59e0ad..b675bc0 100644
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -90,11 +90,6 @@ struct dm_verity_io {
 	 */
 };
 
-struct verity_result {
-	struct completion completion;
-	int err;
-};
-
 static inline struct ahash_request *verity_io_hash_req(struct dm_verity *v,
 						     struct dm_verity_io *io)
 {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 08/11] fscrypt: move to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (6 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 07/11] dm: move dm-verity " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 09/11] cifs: " Gilad Ben-Yossef
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

fscrypt starts several async. crypto ops and waiting for them to
complete. Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 fs/crypto/crypto.c          | 28 ++++------------------------
 fs/crypto/fname.c           | 36 ++++++------------------------------
 fs/crypto/fscrypt_private.h | 10 ----------
 fs/crypto/keyinfo.c         | 21 +++------------------
 4 files changed, 13 insertions(+), 82 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 6d6eca3..227db85 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -125,21 +125,6 @@ struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *inode, gfp_t gfp_flags)
 }
 EXPORT_SYMBOL(fscrypt_get_ctx);
 
-/**
- * page_crypt_complete() - completion callback for page crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void page_crypt_complete(struct crypto_async_request *req, int res)
-{
-	struct fscrypt_completion_result *ecr = req->data;
-
-	if (res == -EINPROGRESS)
-		return;
-	ecr->res = res;
-	complete(&ecr->completion);
-}
-
 int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 			   u64 lblk_num, struct page *src_page,
 			   struct page *dest_page, unsigned int len,
@@ -150,7 +135,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 		u8 padding[FS_XTS_TWEAK_SIZE - sizeof(__le64)];
 	} xts_tweak;
 	struct skcipher_request *req = NULL;
-	DECLARE_FS_COMPLETION_RESULT(ecr);
+	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist dst, src;
 	struct fscrypt_info *ci = inode->i_crypt_info;
 	struct crypto_skcipher *tfm = ci->ci_ctfm;
@@ -168,7 +153,7 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 
 	skcipher_request_set_callback(
 		req, CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-		page_crypt_complete, &ecr);
+		crypto_req_done, &wait);
 
 	BUILD_BUG_ON(sizeof(xts_tweak) != FS_XTS_TWEAK_SIZE);
 	xts_tweak.index = cpu_to_le64(lblk_num);
@@ -180,14 +165,9 @@ int fscrypt_do_page_crypto(const struct inode *inode, fscrypt_direction_t rw,
 	sg_set_page(&src, src_page, len, offs);
 	skcipher_request_set_crypt(req, &src, &dst, len, &xts_tweak);
 	if (rw == FS_DECRYPT)
-		res = crypto_skcipher_decrypt(req);
+		res = crypto_wait_req(crypto_skcipher_decrypt(req), &wait);
 	else
-		res = crypto_skcipher_encrypt(req);
-	if (res == -EINPROGRESS || res == -EBUSY) {
-		BUG_ON(req->base.data != &ecr);
-		wait_for_completion(&ecr.completion);
-		res = ecr.res;
-	}
+		res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 	skcipher_request_free(req);
 	if (res) {
 		printk_ratelimited(KERN_ERR
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index d1bb02b..d5732c9 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -15,21 +15,6 @@
 #include "fscrypt_private.h"
 
 /**
- * fname_crypt_complete() - completion callback for filename crypto
- * @req: The asynchronous cipher request context
- * @res: The result of the cipher operation
- */
-static void fname_crypt_complete(struct crypto_async_request *req, int res)
-{
-	struct fscrypt_completion_result *ecr = req->data;
-
-	if (res == -EINPROGRESS)
-		return;
-	ecr->res = res;
-	complete(&ecr->completion);
-}
-
-/**
  * fname_encrypt() - encrypt a filename
  *
  * The caller must have allocated sufficient memory for the @oname string.
@@ -40,7 +25,7 @@ static int fname_encrypt(struct inode *inode,
 			const struct qstr *iname, struct fscrypt_str *oname)
 {
 	struct skcipher_request *req = NULL;
-	DECLARE_FS_COMPLETION_RESULT(ecr);
+	DECLARE_CRYPTO_WAIT(ecr);
 	struct fscrypt_info *ci = inode->i_crypt_info;
 	struct crypto_skcipher *tfm = ci->ci_ctfm;
 	int res = 0;
@@ -76,17 +61,12 @@ static int fname_encrypt(struct inode *inode,
 	}
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-			fname_crypt_complete, &ecr);
+			crypto_req_done, &ecr);
 	sg_init_one(&sg, oname->name, cryptlen);
 	skcipher_request_set_crypt(req, &sg, &sg, cryptlen, iv);
 
 	/* Do the encryption */
-	res = crypto_skcipher_encrypt(req);
-	if (res == -EINPROGRESS || res == -EBUSY) {
-		/* Request is being completed asynchronously; wait for it */
-		wait_for_completion(&ecr.completion);
-		res = ecr.res;
-	}
+	res = crypto_wait_req(crypto_skcipher_encrypt(req), &ecr);
 	skcipher_request_free(req);
 	if (res < 0) {
 		printk_ratelimited(KERN_ERR
@@ -110,7 +90,7 @@ static int fname_decrypt(struct inode *inode,
 				struct fscrypt_str *oname)
 {
 	struct skcipher_request *req = NULL;
-	DECLARE_FS_COMPLETION_RESULT(ecr);
+	DECLARE_CRYPTO_WAIT(ecr);
 	struct scatterlist src_sg, dst_sg;
 	struct fscrypt_info *ci = inode->i_crypt_info;
 	struct crypto_skcipher *tfm = ci->ci_ctfm;
@@ -131,7 +111,7 @@ static int fname_decrypt(struct inode *inode,
 	}
 	skcipher_request_set_callback(req,
 		CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-		fname_crypt_complete, &ecr);
+		crypto_req_done, &ecr);
 
 	/* Initialize IV */
 	memset(iv, 0, FS_CRYPTO_BLOCK_SIZE);
@@ -140,11 +120,7 @@ static int fname_decrypt(struct inode *inode,
 	sg_init_one(&src_sg, iname->name, iname->len);
 	sg_init_one(&dst_sg, oname->name, oname->len);
 	skcipher_request_set_crypt(req, &src_sg, &dst_sg, iname->len, iv);
-	res = crypto_skcipher_decrypt(req);
-	if (res == -EINPROGRESS || res == -EBUSY) {
-		wait_for_completion(&ecr.completion);
-		res = ecr.res;
-	}
+	res = crypto_wait_req(crypto_skcipher_decrypt(req), &ecr);
 	skcipher_request_free(req);
 	if (res < 0) {
 		printk_ratelimited(KERN_ERR
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 1e1f8a3..b2d130e 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -65,16 +65,6 @@ typedef enum {
 #define FS_CTX_REQUIRES_FREE_ENCRYPT_FL		0x00000001
 #define FS_CTX_HAS_BOUNCE_BUFFER_FL		0x00000002
 
-struct fscrypt_completion_result {
-	struct completion completion;
-	int res;
-};
-
-#define DECLARE_FS_COMPLETION_RESULT(ecr) \
-	struct fscrypt_completion_result ecr = { \
-		COMPLETION_INITIALIZER_ONSTACK((ecr).completion), 0 }
-
-
 /* crypto.c */
 extern int fscrypt_initialize(unsigned int cop_flags);
 extern struct workqueue_struct *fscrypt_read_workqueue;
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 179e578..5661020 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -12,17 +12,6 @@
 #include <linux/scatterlist.h>
 #include "fscrypt_private.h"
 
-static void derive_crypt_complete(struct crypto_async_request *req, int rc)
-{
-	struct fscrypt_completion_result *ecr = req->data;
-
-	if (rc == -EINPROGRESS)
-		return;
-
-	ecr->res = rc;
-	complete(&ecr->completion);
-}
-
 /**
  * derive_key_aes() - Derive a key using AES-128-ECB
  * @deriving_key: Encryption key used for derivation.
@@ -37,7 +26,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 {
 	int res = 0;
 	struct skcipher_request *req = NULL;
-	DECLARE_FS_COMPLETION_RESULT(ecr);
+	DECLARE_CRYPTO_WAIT(wait);
 	struct scatterlist src_sg, dst_sg;
 	struct crypto_skcipher *tfm = crypto_alloc_skcipher("ecb(aes)", 0, 0);
 
@@ -54,7 +43,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	}
 	skcipher_request_set_callback(req,
 			CRYPTO_TFM_REQ_MAY_BACKLOG | CRYPTO_TFM_REQ_MAY_SLEEP,
-			derive_crypt_complete, &ecr);
+			crypto_req_done, &wait);
 	res = crypto_skcipher_setkey(tfm, deriving_key,
 					FS_AES_128_ECB_KEY_SIZE);
 	if (res < 0)
@@ -64,11 +53,7 @@ static int derive_key_aes(u8 deriving_key[FS_AES_128_ECB_KEY_SIZE],
 	sg_init_one(&dst_sg, derived_key, FS_AES_256_XTS_KEY_SIZE);
 	skcipher_request_set_crypt(req, &src_sg, &dst_sg,
 					FS_AES_256_XTS_KEY_SIZE, NULL);
-	res = crypto_skcipher_encrypt(req);
-	if (res == -EINPROGRESS || res == -EBUSY) {
-		wait_for_completion(&ecr.completion);
-		res = ecr.res;
-	}
+	res = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
 out:
 	skcipher_request_free(req);
 	crypto_free_skcipher(tfm);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 09/11] cifs: move to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (7 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 08/11] fscrypt: move " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 10/11] ima: " Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 11/11] crypto: adapt api sample to use async. op wait Gilad Ben-Yossef
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

cifs starts an async. crypto op and waits for their completion.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Pavel Shilovsky <pshilov@microsoft.com>
---
 fs/cifs/smb2ops.c | 30 ++++--------------------------
 1 file changed, 4 insertions(+), 26 deletions(-)

diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index c586918..c0e1882 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -1707,22 +1707,6 @@ init_sg(struct smb_rqst *rqst, u8 *sign)
 	return sg;
 }
 
-struct cifs_crypt_result {
-	int err;
-	struct completion completion;
-};
-
-static void cifs_crypt_complete(struct crypto_async_request *req, int err)
-{
-	struct cifs_crypt_result *res = req->data;
-
-	if (err == -EINPROGRESS)
-		return;
-
-	res->err = err;
-	complete(&res->completion);
-}
-
 static int
 smb2_get_enc_key(struct TCP_Server_Info *server, __u64 ses_id, int enc, u8 *key)
 {
@@ -1763,12 +1747,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	struct aead_request *req;
 	char *iv;
 	unsigned int iv_len;
-	struct cifs_crypt_result result = {0, };
+	DECLARE_CRYPTO_WAIT(wait);
 	struct crypto_aead *tfm;
 	unsigned int crypt_len = le32_to_cpu(tr_hdr->OriginalMessageSize);
 
-	init_completion(&result.completion);
-
 	rc = smb2_get_enc_key(server, tr_hdr->SessionId, enc, key);
 	if (rc) {
 		cifs_dbg(VFS, "%s: Could not get %scryption key\n", __func__,
@@ -1826,14 +1808,10 @@ crypt_message(struct TCP_Server_Info *server, struct smb_rqst *rqst, int enc)
 	aead_request_set_ad(req, assoc_data_len);
 
 	aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-				  cifs_crypt_complete, &result);
+				  crypto_req_done, &wait);
 
-	rc = enc ? crypto_aead_encrypt(req) : crypto_aead_decrypt(req);
-
-	if (rc == -EINPROGRESS || rc == -EBUSY) {
-		wait_for_completion(&result.completion);
-		rc = result.err;
-	}
+	rc = crypto_wait_req(enc ? crypto_aead_encrypt(req)
+				: crypto_aead_decrypt(req), &wait);
 
 	if (!rc && enc)
 		memcpy(&tr_hdr->Signature, sign, SMB2_SIGNATURE_SIZE);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 10/11] ima: move to generic async completion
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (8 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 09/11] cifs: " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  2017-05-29  8:22 ` [PATCH v2 11/11] crypto: adapt api sample to use async. op wait Gilad Ben-Yossef
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

ima starts several async crypto ops and  waits for their completions.
Move it over to generic code doing the same.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
Acked-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_crypto.c | 56 +++++++++++--------------------------
 1 file changed, 17 insertions(+), 39 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 802d5d2..0e4db1fe 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -27,11 +27,6 @@
 
 #include "ima.h"
 
-struct ahash_completion {
-	struct completion completion;
-	int err;
-};
-
 /* minimum file size for ahash use */
 static unsigned long ima_ahash_minsize;
 module_param_named(ahash_minsize, ima_ahash_minsize, ulong, 0644);
@@ -196,30 +191,13 @@ static void ima_free_atfm(struct crypto_ahash *tfm)
 		crypto_free_ahash(tfm);
 }
 
-static void ahash_complete(struct crypto_async_request *req, int err)
+static inline int ahash_wait(int err, struct crypto_wait *wait)
 {
-	struct ahash_completion *res = req->data;
 
-	if (err == -EINPROGRESS)
-		return;
-	res->err = err;
-	complete(&res->completion);
-}
+	err = crypto_wait_req(err, wait);
 
-static int ahash_wait(int err, struct ahash_completion *res)
-{
-	switch (err) {
-	case 0:
-		break;
-	case -EINPROGRESS:
-	case -EBUSY:
-		wait_for_completion(&res->completion);
-		reinit_completion(&res->completion);
-		err = res->err;
-		/* fall through */
-	default:
+	if (err)
 		pr_crit_ratelimited("ahash calculation failed: err: %d\n", err);
-	}
 
 	return err;
 }
@@ -233,7 +211,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	int rc, read = 0, rbuf_len, active = 0, ahash_rc = 0;
 	struct ahash_request *req;
 	struct scatterlist sg[1];
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	size_t rbuf_size[2];
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -242,12 +220,12 @@ static int ima_calc_file_hash_atfm(struct file *file,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out1;
 
@@ -288,7 +266,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -304,7 +282,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			 * read/request, wait for the completion of the
 			 * previous ahash_update() request.
 			 */
-			rc = ahash_wait(ahash_rc, &res);
+			rc = ahash_wait(ahash_rc, &wait);
 			if (rc)
 				goto out3;
 		}
@@ -318,7 +296,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 			active = !active; /* swap buffers, if we use two */
 	}
 	/* wait for the last update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 out3:
 	if (read)
 		file->f_mode &= ~FMODE_READ;
@@ -327,7 +305,7 @@ static int ima_calc_file_hash_atfm(struct file *file,
 out2:
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out1:
 	ahash_request_free(req);
@@ -527,7 +505,7 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 {
 	struct ahash_request *req;
 	struct scatterlist sg;
-	struct ahash_completion res;
+	struct crypto_wait wait;
 	int rc, ahash_rc = 0;
 
 	hash->length = crypto_ahash_digestsize(tfm);
@@ -536,12 +514,12 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	if (!req)
 		return -ENOMEM;
 
-	init_completion(&res.completion);
+	crypto_init_wait(&wait);
 	ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
 				   CRYPTO_TFM_REQ_MAY_SLEEP,
-				   ahash_complete, &res);
+				   crypto_req_done, &wait);
 
-	rc = ahash_wait(crypto_ahash_init(req), &res);
+	rc = ahash_wait(crypto_ahash_init(req), &wait);
 	if (rc)
 		goto out;
 
@@ -551,10 +529,10 @@ static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
 	ahash_rc = crypto_ahash_update(req);
 
 	/* wait for the update request to complete */
-	rc = ahash_wait(ahash_rc, &res);
+	rc = ahash_wait(ahash_rc, &wait);
 	if (!rc) {
 		ahash_request_set_crypt(req, NULL, hash->digest, 0);
-		rc = ahash_wait(crypto_ahash_final(req), &res);
+		rc = ahash_wait(crypto_ahash_final(req), &wait);
 	}
 out:
 	ahash_request_free(req);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 11/11] crypto: adapt api sample to use async. op wait
  2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
                   ` (9 preceding siblings ...)
  2017-05-29  8:22 ` [PATCH v2 10/11] ima: " Gilad Ben-Yossef
@ 2017-05-29  8:22 ` Gilad Ben-Yossef
  10 siblings, 0 replies; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-05-29  8:22 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
	Alasdair Kergon, Mike Snitzer, dm-devel, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn
  Cc: Eric Biggers, Ofir Drang, Pavel Shilovsky, linux-crypto,
	linux-doc, linux-kernel, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

The code sample is waiting for an async. crypto op completion.
Adapt sample to use the new generic infrastructure to do the same.

This also fixes a possible data coruption bug created by the
use of wait_for_completion_interruptible() without dealing
correctly with an interrupt aborting the wait prior to the
async op finishing.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 Documentation/crypto/api-samples.rst | 52 +++++++-----------------------------
 1 file changed, 10 insertions(+), 42 deletions(-)

diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index d021fd9..73b466b 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -7,59 +7,27 @@ Code Example For Symmetric Key Cipher Operation
 ::
 
 
-    struct tcrypt_result {
-        struct completion completion;
-        int err;
-    };
-
     /* tie all data structures together */
     struct skcipher_def {
         struct scatterlist sg;
         struct crypto_skcipher *tfm;
         struct skcipher_request *req;
-        struct tcrypt_result result;
+        struct crypto_wait wait;
     };
 
-    /* Callback function */
-    static void test_skcipher_cb(struct crypto_async_request *req, int error)
-    {
-        struct tcrypt_result *result = req->data;
-
-        if (error == -EINPROGRESS)
-            return;
-        result->err = error;
-        complete(&result->completion);
-        pr_info("Encryption finished successfully\n");
-    }
-
     /* Perform cipher operation */
     static unsigned int test_skcipher_encdec(struct skcipher_def *sk,
                          int enc)
     {
-        int rc = 0;
+        int rc;
 
         if (enc)
-            rc = crypto_skcipher_encrypt(sk->req);
+            rc = crypto_wait_req(crypto_skcipher_encrypt(sk->req), &sk->wait);
         else
-            rc = crypto_skcipher_decrypt(sk->req);
-
-        switch (rc) {
-        case 0:
-            break;
-        case -EINPROGRESS:
-        case -EBUSY:
-            rc = wait_for_completion_interruptible(
-                &sk->result.completion);
-            if (!rc && !sk->result.err) {
-                reinit_completion(&sk->result.completion);
-                break;
-            }
-        default:
-            pr_info("skcipher encrypt returned with %d result %d\n",
-                rc, sk->result.err);
-            break;
-        }
-        init_completion(&sk->result.completion);
+            rc = crypto_wait_req(crypto_skcipher_decrypt(sk->req), &sk->wait);
+
+	if (rc)
+		pr_info("skcipher encrypt returned with result %d\n", rc);
 
         return rc;
     }
@@ -89,8 +57,8 @@ Code Example For Symmetric Key Cipher Operation
         }
 
         skcipher_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
-                          test_skcipher_cb,
-                          &sk.result);
+                          crypto_req_done,
+                          &sk.wait);
 
         /* AES 256 with random key */
         get_random_bytes(&key, 32);
@@ -122,7 +90,7 @@ Code Example For Symmetric Key Cipher Operation
         /* We encrypt one block */
         sg_init_one(&sk.sg, scratchpad, 16);
         skcipher_request_set_crypt(req, &sk.sg, &sk.sg, 16, ivdata);
-        init_completion(&sk.result.completion);
+        crypto_init_wait(&sk.wait);
 
         /* encrypt data */
         ret = test_skcipher_encdec(&sk, 1);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op
  2017-05-29  8:22 ` [PATCH v2 01/11] crypto: " Gilad Ben-Yossef
@ 2017-06-10  3:43   ` Herbert Xu
  2017-06-10  8:05     ` Gilad Ben-Yossef
  0 siblings, 1 reply; 15+ messages in thread
From: Herbert Xu @ 2017-06-10  3:43 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: David S. Miller, Jonathan Corbet, David Howells, Alasdair Kergon,
	Mike Snitzer, dm-devel, Shaohua Li, Steve French,
	Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar, Dmitry Kasatkin,
	James Morris, Serge E. Hallyn, Eric Biggers, Ofir Drang,
	Pavel Shilovsky, linux-crypto, linux-doc, linux-kernel, keyrings,
	linux-raid, linux-cifs, samba-technical, linux-fscrypt,
	linux-ima-devel, linux-ima-user, linux-security-module

On Mon, May 29, 2017 at 11:22:48AM +0300, Gilad Ben-Yossef wrote:
>
> +static inline int crypto_wait_req(int err, struct crypto_wait *wait)
> +{
> +	switch (err) {
> +	case -EINPROGRESS:
> +	case -EBUSY:
> +		wait_for_completion(&wait->completion);
> +		reinit_completion(&wait->completion);
> +		err = wait->err;
> +		break;
> +	};
> +
> +	return err;
> +}

This assumes that the request is used with backlog.  For non-backlog
requests this would result in a memory leak as EBUSY in that case is
a fatal error.

So this API can't be used without backlog.

We could introduce a flag to indicate whether we want backlog or not,
or maybe we should change our API so that in the non-backlog case we
return something other than EBUSY.

Opinions?

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op
  2017-06-10  3:43   ` Herbert Xu
@ 2017-06-10  8:05     ` Gilad Ben-Yossef
  2017-06-10  9:01       ` Herbert Xu
  0 siblings, 1 reply; 15+ messages in thread
From: Gilad Ben-Yossef @ 2017-06-10  8:05 UTC (permalink / raw)
  To: Herbert Xu
  Cc: David S. Miller, Jonathan Corbet, David Howells, Alasdair Kergon,
	Mike Snitzer, device-mapper development, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn, Eric Biggers,
	Ofir Drang, Pavel Shilovsky, linux-crypto, linux-doc,
	Linux kernel mailing list, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

On Sat, Jun 10, 2017 at 6:43 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Mon, May 29, 2017 at 11:22:48AM +0300, Gilad Ben-Yossef wrote:
>>
>> +static inline int crypto_wait_req(int err, struct crypto_wait *wait)
>> +{
>> +     switch (err) {
>> +     case -EINPROGRESS:
>> +     case -EBUSY:
>> +             wait_for_completion(&wait->completion);
>> +             reinit_completion(&wait->completion);
>> +             err = wait->err;
>> +             break;
>> +     };
>> +
>> +     return err;
>> +}
>
> This assumes that the request is used with backlog.  For non-backlog
> requests this would result in a memory leak as EBUSY in that case is
> a fatal error.
>
> So this API can't be used without backlog.

You are right, of course. I did not take that into account.

>
> We could introduce a flag to indicate whether we want backlog or not,
> or maybe we should change our API so that in the non-backlog case we
> return something other than EBUSY.
>
> Opinions?

I guess there is a question if it really is important to know that
your request ended up
on the backlog, rather than being handled.I can imagine it can be used
as back pressure
indication but I wonder if someone is using that.

If not, maybe we can simplify things and use EINPROGRESS asindication
of a request
being accepted by the next layer (either being processed or queued in
the back log), whereas
EBUSY would indicate failure.

It does have a potential to make things simpler, I think.

Gilad

>
> 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



-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 01/11] crypto: introduce crypto wait for async op
  2017-06-10  8:05     ` Gilad Ben-Yossef
@ 2017-06-10  9:01       ` Herbert Xu
  0 siblings, 0 replies; 15+ messages in thread
From: Herbert Xu @ 2017-06-10  9:01 UTC (permalink / raw)
  To: Gilad Ben-Yossef
  Cc: David S. Miller, Jonathan Corbet, David Howells, Alasdair Kergon,
	Mike Snitzer, device-mapper development, Shaohua Li,
	Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Mimi Zohar,
	Dmitry Kasatkin, James Morris, Serge E. Hallyn, Eric Biggers,
	Ofir Drang, Pavel Shilovsky, linux-crypto, linux-doc,
	Linux kernel mailing list, keyrings, linux-raid, linux-cifs,
	samba-technical, linux-fscrypt, linux-ima-devel, linux-ima-user,
	linux-security-module

On Sat, Jun 10, 2017 at 11:05:39AM +0300, Gilad Ben-Yossef wrote:
>
> I guess there is a question if it really is important to know that
> your request ended up
> on the backlog, rather than being handled.I can imagine it can be used
> as back pressure
> indication but I wonder if someone is using that.

Oh yes we do want it to return EBUSY if we put it on the backlog
because in that case we want the user to stop sending us new
requests.

It's the other case where we dropped the request and returned
EBUSY where I think we could return something other than EBUSY
and get rid of the ambiguity.

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-06-10  9:03 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-29  8:22 [PATCH v2 00/11] introduce crypto wait for async op Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 01/11] crypto: " Gilad Ben-Yossef
2017-06-10  3:43   ` Herbert Xu
2017-06-10  8:05     ` Gilad Ben-Yossef
2017-06-10  9:01       ` Herbert Xu
2017-05-29  8:22 ` [PATCH v2 02/11] crypto: move algif to generic async completion Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 03/11] crypto: move pub key " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 04/11] crypto: move drbg " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 05/11] crypto: move gcm " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 06/11] crypto: move testmgr " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 07/11] dm: move dm-verity " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 08/11] fscrypt: move " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 09/11] cifs: " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 10/11] ima: " Gilad Ben-Yossef
2017-05-29  8:22 ` [PATCH v2 11/11] crypto: adapt api sample to use async. op wait Gilad Ben-Yossef

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).