[PATCH v2 0/4] crypto: async crypto op fixes

[PATCH v2 0/4] crypto: async crypto op fixes

[Gilad Ben-Yossef]

This patch set fixes various usage and documentation errors
in waiting for async crypto op to complete which can result
in data corruption.

Note: these were discovered in the process of working on a
patch set that replaces these call sites and more with a
generic implementation that will prevent these problems
going forward. These are just the fix ups for current code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: stable@vger.kernel.org
CC: Eric Biggers <ebiggers3@gmail.com>

Gilad Ben-Yossef (4):
  crypto: handle EBUSY due to backlog correctly
  crypto: drbg wait for crypto op not signal safe
  crypto: gcm wait for crypto op not signal safe
  crypto: Documentation: fix none signal safe sample

Changes from v1:
- Fix a brown paper bug compile error due to bad git rebase squashing

 Documentation/crypto/api-samples.rst | 2 +-
 crypto/asymmetric_keys/public_key.c  | 2 +-
 crypto/drbg.c                        | 5 ++---
 crypto/gcm.c                         | 6 ++----
 4 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.1.4

[PATCH v2 1/4] crypto: handle EBUSY due to backlog correctly

[Gilad Ben-Yossef]

public_key_verify_signature() was passing the CRYPTO_TFM_REQ_MAY_BACKLOG
flag to akcipher_request_set_callback() but was not handling correctly
the case where a -EBUSY error could be returned from the call to
crypto_akcipher_verify() if backlog was used, possibly casuing
data corruption due to use-after-free of buffers.

Resolve this by handling -EBUSY correctly.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: stable@vger.kernel.org
---
 crypto/asymmetric_keys/public_key.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c
index d3a989e..3cd6e12 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) {
+	if ((ret == -EINPROGRESS) || (ret == -EBUSY)) {
 		wait_for_completion(&compl.completion);
 		ret = compl.err;
 	}
-- 
2.1.4

[PATCH v2 2/4] crypto: drbg wait for crypto op not signal safe

[Gilad Ben-Yossef]

drbg_kcapi_sym_ctr() was using wait_for_completion_interruptible() to
wait for completion of async crypto op but if a signal occurs it
may return before DMA ops of HW crypto provider finish, thus
corrupting the output buffer.

Resolve this by using wait_for_completion() instead.

Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: stable@vger.kernel.org
---
 crypto/drbg.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index fa749f4..cdb27ac 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -1767,9 +1767,8 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 			break;
 		case -EINPROGRESS:
 		case -EBUSY:
-			ret = wait_for_completion_interruptible(
-				&drbg->ctr_completion);
-			if (!ret && !drbg->ctr_async_err) {
+			wait_for_completion(&drbg->ctr_completion);
+			if (!drbg->ctr_async_err) {
 				reinit_completion(&drbg->ctr_completion);
 				break;
 			}
-- 
2.1.4

[PATCH v2 3/4] crypto: gcm wait for crypto op not signal safe

[Gilad Ben-Yossef]

crypto_gcm_setkey() was using wait_for_completion_interruptible() to
wait for completion of async crypto op but if a signal occurs it
may return before DMA ops of HW crypto provider finish, thus
corrupting the data buffer that is kfree'ed in this case.

Resolve this by using wait_for_completion() instead.

Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: stable@vger.kernel.org
---
 crypto/gcm.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/crypto/gcm.c b/crypto/gcm.c
index b7ad808..3841b5e 100644
--- a/crypto/gcm.c
+++ b/crypto/gcm.c
@@ -152,10 +152,8 @@ static int crypto_gcm_setkey(struct crypto_aead *aead, const u8 *key,
 
 	err = crypto_skcipher_encrypt(&data->req);
 	if (err == -EINPROGRESS || err == -EBUSY) {
-		err = wait_for_completion_interruptible(
-			&data->result.completion);
-		if (!err)
-			err = data->result.err;
+		wait_for_completion(&data->result.completion);
+		err = data->result.err;
 	}
 
 	if (err)
-- 
2.1.4

[PATCH v2 4/4] crypto: Documentation: fix none signal safe sample

[Gilad Ben-Yossef]

The sample code was showing use of wait_for_completion_interruptible()
for waiting for an async. crypto op to finish. However, if a signal
arrived it would free the buffers used even while crypto HW might
still DMA from/into the buffers.

Resolve this by using wait_for_completion() instead.

Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
---
 Documentation/crypto/api-samples.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/crypto/api-samples.rst b/Documentation/crypto/api-samples.rst
index d021fd9..944f08b 100644
--- a/Documentation/crypto/api-samples.rst
+++ b/Documentation/crypto/api-samples.rst
@@ -48,7 +48,7 @@ Code Example For Symmetric Key Cipher Operation
             break;
         case -EINPROGRESS:
         case -EBUSY:
-            rc = wait_for_completion_interruptible(
+            rc = wait_for_completion(
                 &sk->result.completion);
             if (!rc && !sk->result.err) {
                 reinit_completion(&sk->result.completion);
-- 
2.1.4

Re: [PATCH v2 0/4] crypto: async crypto op fixes

[Herbert Xu]

On Thu, May 18, 2017 at 04:29:22PM +0300, Gilad Ben-Yossef wrote:
> This patch set fixes various usage and documentation errors
> in waiting for async crypto op to complete which can result
> in data corruption.
> 
> Note: these were discovered in the process of working on a
> patch set that replaces these call sites and more with a
> generic implementation that will prevent these problems
> going forward. These are just the fix ups for current code.
> 
> Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
> CC: stable@vger.kernel.org
> CC: Eric Biggers <ebiggers3@gmail.com>

Patches 1-3 applied.  Please fix patch 4 and resubmit.  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

[PATCH v2 0/4] crypto: async crypto op fixes

[Gilad Ben-Yossef]

This patch set fixes various usage and documentation errors
in waiting for async crypto op to complete which can result
in data corruption.

Note: these were discovered in the process of working on a
patch set that replaces these call sites and more with a
generic implementation that will prevent these problems
going forward. These are just the fix ups for current code.

Signed-off-by: Gilad Ben-Yossef <gilad@benyossef.com>
CC: stable@vger.kernel.org
CC: Eric Biggers <ebiggers3@gmail.com>

Gilad Ben-Yossef (4):
  crypto: handle EBUSY due to backlog correctly
  crypto: drbg wait for crypto op not signal safe
  crypto: gcm wait for crypto op not signal safe
  crypto: Documentation: fix none signal safe sample

Changes from v1:
- Fix a brown paper bug compile error due to bad git rebase squashing

 Documentation/crypto/api-samples.rst | 2 +-
 crypto/asymmetric_keys/public_key.c  | 2 +-
 crypto/drbg.c                        | 5 ++---
 crypto/gcm.c                         | 6 ++----
 4 files changed, 6 insertions(+), 9 deletions(-)

-- 
2.1.4