diff mbox

[v3,01/28] crypto: change backlog return code to -EIOCBQUEUED

Message ID 1499006535-19760-2-git-send-email-gilad@benyossef.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Gilad Ben-Yossef July 2, 2017, 2:41 p.m. UTC 
The crypto API was using the -EBUSY return value to indicate
both a hard failure to submit a crypto operation into a
transformation provider when the latter was busy and the backlog
mechanism was not enabled as well as a notification that the operation
was queued into the backlog when the backlog mechanism was enabled.

Having the same return code indicate two very different conditions
depending on a flag is both error prone and requires extra runtime
check like the following to discern between the cases:

	if (err == -EINPROGRESS ||
	    (err == -EBUSY && (ahash_request_flags(req) &
			       CRYPTO_TFM_REQ_MAY_BACKLOG)))

This patch changes the return code used to indicate a crypto op
was queued in the backlog to -EIOCBQUEUED, thus resolving both
issues.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 crypto/af_alg.c                     |  2 +-
 crypto/ahash.c                      | 12 +++---------
 crypto/algapi.c                     |  6 ++++--
 crypto/asymmetric_keys/public_key.c |  2 +-
 crypto/chacha20poly1305.c           |  2 +-
 crypto/cryptd.c                     |  4 +---
 crypto/cts.c                        |  6 ++----
 crypto/drbg.c                       |  2 +-
 crypto/gcm.c                        |  2 +-
 crypto/lrw.c                        |  8 ++------
 crypto/rsa-pkcs1pad.c               | 16 ++++------------
 crypto/tcrypt.c                     |  6 +++---
 crypto/testmgr.c                    | 12 ++++++------
 crypto/xts.c                        |  8 ++------
 14 files changed, 32 insertions(+), 56 deletions(-)

Comments

Herbert Xu July 3, 2017, 12:35 p.m. UTC | #1 
On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
> The crypto API was using the -EBUSY return value to indicate
> both a hard failure to submit a crypto operation into a
> transformation provider when the latter was busy and the backlog
> mechanism was not enabled as well as a notification that the operation
> was queued into the backlog when the backlog mechanism was enabled.
> 
> Having the same return code indicate two very different conditions
> depending on a flag is both error prone and requires extra runtime
> check like the following to discern between the cases:
> 
> 	if (err == -EINPROGRESS ||
> 	    (err == -EBUSY && (ahash_request_flags(req) &
> 			       CRYPTO_TFM_REQ_MAY_BACKLOG)))
> 
> This patch changes the return code used to indicate a crypto op
> was queued in the backlog to -EIOCBQUEUED, thus resolving both
> issues.
> 
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>

So you're changing the success case from EBUSY to EIOCBQUEUED.
This results in a lot of churn as you have to change every single
driver and caller.

How about changing the error case to use something other than
EBUSY instead?

Thanks,
Gilad Ben-Yossef July 3, 2017, 1:23 p.m. UTC | #2 
On Mon, Jul 3, 2017 at 3:35 PM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Sun, Jul 02, 2017 at 05:41:43PM +0300, Gilad Ben-Yossef wrote:
>> The crypto API was using the -EBUSY return value to indicate
>> both a hard failure to submit a crypto operation into a
>> transformation provider when the latter was busy and the backlog
>> mechanism was not enabled as well as a notification that the operation
>> was queued into the backlog when the backlog mechanism was enabled.
>>
>> Having the same return code indicate two very different conditions
>> depending on a flag is both error prone and requires extra runtime
>> check like the following to discern between the cases:
>>
>>       if (err == -EINPROGRESS ||
>>           (err == -EBUSY && (ahash_request_flags(req) &
>>                              CRYPTO_TFM_REQ_MAY_BACKLOG)))
>>
>> This patch changes the return code used to indicate a crypto op
>> was queued in the backlog to -EIOCBQUEUED, thus resolving both
>> issues.
>>
>> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
>
> So you're changing the success case from EBUSY to EIOCBQUEUED.
> This results in a lot of churn as you have to change every single
> driver and caller.
>
> How about changing the error case to use something other than
> EBUSY instead?

That was indeed my first thought - I wanted to change EBUSY to EAGAIN.
It might even be a more descriptive error message since the failure is
transient.

What stopped me was that I saw the algif interface passes this EBUSY value
to user space. I don't know of any software that depends on this value but
I'm really averse to changing user space API.

Of course, we can just plant a (ret != AGAIN : ? EBUSY) in the algif interface
return to user space code path but that felt like a kludge to me.

And it doesn't really save any churn, I think - I'd still have to
visit all the places that
use the flags value to distinguish between the two EBUSY use cases and  I need
to visit most places that check for EBUSY as backlog indication because I'm
replacing the check with the generic replacement, so it doesn't look
like in the end
it will be a smaller change.

Having said that, if you prefer me to replace the error case I'd
happily do that.

Thanks,
Gilad
diff mbox

Patch

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 3556d8e..c67daba 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -484,7 +484,7 @@  int af_alg_wait_for_completion(int err, struct af_alg_completion *completion)
 {
 	switch (err) {
 	case -EINPROGRESS:
-	case -EBUSY:
+	case -EIOCBQUEUED:
 		wait_for_completion(&completion->completion);
 		reinit_completion(&completion->completion);
 		err = completion->err;
diff --git a/crypto/ahash.c b/crypto/ahash.c
index 826cd7a..65d08db 100644
--- a/crypto/ahash.c
+++ b/crypto/ahash.c
@@ -334,9 +334,7 @@  static int ahash_op_unaligned(struct ahash_request *req,
 		return err;
 
 	err = op(req);
-	if (err == -EINPROGRESS ||
-	    (err == -EBUSY && (ahash_request_flags(req) &
-			       CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 		return err;
 
 	ahash_restore_req(req, err);
@@ -394,9 +392,7 @@  static int ahash_def_finup_finish1(struct ahash_request *req, int err)
 	req->base.complete = ahash_def_finup_done2;
 
 	err = crypto_ahash_reqtfm(req)->final(req);
-	if (err == -EINPROGRESS ||
-	    (err == -EBUSY && (ahash_request_flags(req) &
-			       CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 		return err;
 
 out:
@@ -432,9 +428,7 @@  static int ahash_def_finup(struct ahash_request *req)
 		return err;
 
 	err = tfm->update(req);
-	if (err == -EINPROGRESS ||
-	    (err == -EBUSY && (ahash_request_flags(req) &
-			       CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 		return err;
 
 	return ahash_def_finup_finish1(req, err);
diff --git a/crypto/algapi.c b/crypto/algapi.c
index e4cc761..3bfd1fa 100644
--- a/crypto/algapi.c
+++ b/crypto/algapi.c
@@ -897,9 +897,11 @@  int crypto_enqueue_request(struct crypto_queue *queue,
 	int err = -EINPROGRESS;
 
 	if (unlikely(queue->qlen >= queue->max_qlen)) {
-		err = -EBUSY;
-		if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+		if (!(request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG)) {
+			err = -EBUSY;
 			goto out;
+		}
+		err = -EIOCBQUEUED;
 		if (queue->backlog == &queue->list)
 			queue->backlog = &request->list;
 	}
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index 3cd6e12..3fad1fd 100644
--- a/crypto/asymmetric_keys/public_key.c
+++ b/crypto/asymmetric_keys/public_key.c
@@ -141,7 +141,7 @@  int public_key_verify_signature(const struct public_key *pkey,
 	 * signature and returns that to us.
 	 */
 	ret = crypto_akcipher_verify(req);
-	if ((ret == -EINPROGRESS) || (ret == -EBUSY)) {
+	if ((ret == -EINPROGRESS) || (ret == -EIOCBQUEUED)) {
 		wait_for_completion(&compl.completion);
 		ret = compl.err;
 	}
diff --git a/crypto/chacha20poly1305.c b/crypto/chacha20poly1305.c
index db1bc31..e0e2785 100644
--- a/crypto/chacha20poly1305.c
+++ b/crypto/chacha20poly1305.c
@@ -79,7 +79,7 @@  static inline void async_done_continue(struct aead_request *req, int err,
 	if (!err)
 		err = cont(req);
 
-	if (err != -EINPROGRESS && err != -EBUSY)
+	if (err != -EINPROGRESS && err != -EIOCBQUEUED)
 		aead_request_complete(req, err);
 }
 
diff --git a/crypto/cryptd.c b/crypto/cryptd.c
index 0508c48..5daca13 100644
--- a/crypto/cryptd.c
+++ b/crypto/cryptd.c
@@ -137,16 +137,14 @@  static int cryptd_enqueue_request(struct cryptd_queue *queue,
 	int cpu, err;
 	struct cryptd_cpu_queue *cpu_queue;
 	atomic_t *refcnt;
-	bool may_backlog;
 
 	cpu = get_cpu();
 	cpu_queue = this_cpu_ptr(queue->cpu_queue);
 	err = crypto_enqueue_request(&cpu_queue->queue, request);
 
 	refcnt = crypto_tfm_ctx(request->tfm);
-	may_backlog = request->flags & CRYPTO_TFM_REQ_MAY_BACKLOG;
 
-	if (err == -EBUSY && !may_backlog)
+	if (err == -EBUSY)
 		goto out_put_cpu;
 
 	queue_work_on(cpu, kcrypto_wq, &cpu_queue->work);
diff --git a/crypto/cts.c b/crypto/cts.c
index 243f591..e2068dc 100644
--- a/crypto/cts.c
+++ b/crypto/cts.c
@@ -136,8 +136,7 @@  static void crypto_cts_encrypt_done(struct crypto_async_request *areq, int err)
 		goto out;
 
 	err = cts_cbc_encrypt(req);
-	if (err == -EINPROGRESS ||
-	    (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 		return;
 
 out:
@@ -229,8 +228,7 @@  static void crypto_cts_decrypt_done(struct crypto_async_request *areq, int err)
 		goto out;
 
 	err = cts_cbc_decrypt(req);
-	if (err == -EINPROGRESS ||
-	    (err == -EBUSY && req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 		return;
 
 out:
diff --git a/crypto/drbg.c b/crypto/drbg.c
index 633a88e..850b451 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1767,7 +1767,7 @@  static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 		case 0:
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&drbg->ctr_completion);
 			if (!drbg->ctr_async_err) {
 				reinit_completion(&drbg->ctr_completion);
diff --git a/crypto/gcm.c b/crypto/gcm.c
index 3841b5e..ffac821 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -151,7 +151,7 @@  static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 				   sizeof(data->hash), data->iv);
 
 	err = crypto_skcipher_encrypt(&data->req);
-	if (err == -EINPROGRESS || err == -EBUSY) {
+	if (err == -EINPROGRESS || err == -EIOCBQUEUED) {
 		wait_for_completion(&data->result.completion);
 		err = data->result.err;
 	}
diff --git a/crypto/lrw.c b/crypto/lrw.c
index a8bfae4..e2b1378 100644
--- a/crypto/lrw.c
+++ b/crypto/lrw.c
@@ -328,9 +328,7 @@  static int do_encrypt(struct skcipher_request *req, int err)
 		      crypto_skcipher_encrypt(subreq) ?:
 		      post_crypt(req);
 
-		if (err == -EINPROGRESS ||
-		    (err == -EBUSY &&
-		     req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+		if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 			return err;
 	}
 
@@ -380,9 +378,7 @@  static int do_decrypt(struct skcipher_request *req, int err)
 		      crypto_skcipher_decrypt(subreq) ?:
 		      post_crypt(req);
 
-		if (err == -EINPROGRESS ||
-		    (err == -EBUSY &&
-		     req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+		if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 			return err;
 	}
 
diff --git a/crypto/rsa-pkcs1pad.c b/crypto/rsa-pkcs1pad.c
index 407c64b..3c82542 100644
--- a/crypto/rsa-pkcs1pad.c
+++ b/crypto/rsa-pkcs1pad.c
@@ -279,9 +279,7 @@  static int pkcs1pad_encrypt(struct akcipher_request *req)
 				   req->dst, ctx->key_size - 1, req->dst_len);
 
 	err = crypto_akcipher_encrypt(&req_ctx->child_req);
-	if (err != -EINPROGRESS &&
-			(err != -EBUSY ||
-			 !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err != -EINPROGRESS && err != -EIOCBQUEUED)
 		return pkcs1pad_encrypt_sign_complete(req, err);
 
 	return err;
@@ -383,9 +381,7 @@  static int pkcs1pad_decrypt(struct akcipher_request *req)
 				   ctx->key_size);
 
 	err = crypto_akcipher_decrypt(&req_ctx->child_req);
-	if (err != -EINPROGRESS &&
-			(err != -EBUSY ||
-			 !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err != -EINPROGRESS && err != -EIOCBQUEUED)
 		return pkcs1pad_decrypt_complete(req, err);
 
 	return err;
@@ -440,9 +436,7 @@  static int pkcs1pad_sign(struct akcipher_request *req)
 				   req->dst, ctx->key_size - 1, req->dst_len);
 
 	err = crypto_akcipher_sign(&req_ctx->child_req);
-	if (err != -EINPROGRESS &&
-			(err != -EBUSY ||
-			 !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err != -EINPROGRESS && err != -EIOCBQUEUED)
 		return pkcs1pad_encrypt_sign_complete(req, err);
 
 	return err;
@@ -561,9 +555,7 @@  static int pkcs1pad_verify(struct akcipher_request *req)
 				   ctx->key_size);
 
 	err = crypto_akcipher_verify(&req_ctx->child_req);
-	if (err != -EINPROGRESS &&
-			(err != -EBUSY ||
-			 !(req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG)))
+	if (err != -EINPROGRESS && err != -EIOCBQUEUED)
 		return pkcs1pad_verify_complete(req, err);
 
 	return err;
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 0dd6a43..57f7ac4 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -97,7 +97,7 @@  static void tcrypt_complete(struct crypto_async_request *req, int err)
 
 static inline int do_one_aead_op(struct aead_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
+	if (ret == -EINPROGRESS || ret == -EIOCBQUEUED) {
 		struct tcrypt_result *tr = req->base.data;
 
 		ret = wait_for_completion_interruptible(&tr->completion);
@@ -397,7 +397,7 @@  static void test_hash_sg_init(struct scatterlist *sg)
 
 static inline int do_one_ahash_op(struct ahash_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
+	if (ret == -EINPROGRESS || ret == -EIOCBQUEUED) {
 		struct tcrypt_result *tr = req->base.data;
 
 		wait_for_completion(&tr->completion);
@@ -765,7 +765,7 @@  static void test_hash_speed(const char *algo, unsigned int secs,
 
 static inline int do_one_acipher_op(struct skcipher_request *req, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
+	if (ret == -EINPROGRESS || ret == -EIOCBQUEUED) {
 		struct tcrypt_result *tr = req->base.data;
 
 		wait_for_completion(&tr->completion);
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 7125ba3..fb5418f 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -195,7 +195,7 @@  static void testmgr_free_buf(char *buf[XBUFSIZE])
 
 static int wait_async_op(struct tcrypt_result *tr, int ret)
 {
-	if (ret == -EINPROGRESS || ret == -EBUSY) {
+	if (ret == -EINPROGRESS || ret == -EIOCBQUEUED) {
 		wait_for_completion(&tr->completion);
 		reinit_completion(&tr->completion);
 		ret = tr->err;
@@ -425,7 +425,7 @@  static int __test_hash(struct crypto_ahash *tfm,
 		case 0:
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&tresult.completion);
 			reinit_completion(&tresult.completion);
 			ret = tresult.err;
@@ -723,7 +723,7 @@  static int __test_aead(struct crypto_aead *tfm, int enc,
 			}
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&result.completion);
 			reinit_completion(&result.completion);
 			ret = result.err;
@@ -880,7 +880,7 @@  static int __test_aead(struct crypto_aead *tfm, int enc,
 			}
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&result.completion);
 			reinit_completion(&result.completion);
 			ret = result.err;
@@ -1171,7 +1171,7 @@  static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 		case 0:
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&result.completion);
 			reinit_completion(&result.completion);
 			ret = result.err;
@@ -1279,7 +1279,7 @@  static int __test_skcipher(struct crypto_skcipher *tfm, int enc,
 		case 0:
 			break;
 		case -EINPROGRESS:
-		case -EBUSY:
+		case -EIOCBQUEUED:
 			wait_for_completion(&result.completion);
 			reinit_completion(&result.completion);
 			ret = result.err;
diff --git a/crypto/xts.c b/crypto/xts.c
index d86c11a..b4b90cc 100644
--- a/crypto/xts.c
+++ b/crypto/xts.c
@@ -269,9 +269,7 @@  static int do_encrypt(struct skcipher_request *req, int err)
 		      crypto_skcipher_encrypt(subreq) ?:
 		      post_crypt(req);
 
-		if (err == -EINPROGRESS ||
-		    (err == -EBUSY &&
-		     req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+		if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 			return err;
 	}
 
@@ -321,9 +319,7 @@  static int do_decrypt(struct skcipher_request *req, int err)
 		      crypto_skcipher_decrypt(subreq) ?:
 		      post_crypt(req);
 
-		if (err == -EINPROGRESS ||
-		    (err == -EBUSY &&
-		     req->base.flags & CRYPTO_TFM_REQ_MAY_BACKLOG))
+		if (err == -EINPROGRESS || err == -EIOCBQUEUED)
 			return err;
 	}