The crypto API wants the updated IV in req->info after decryption. The
updated IV used to be copied correctly to req->info after running the
decryption job. Since 115957bb3e59 this is done before running the job
so instead of the updated IV only the unmodified input IV is given back
to the crypto API.
This was observed running the gcm(aes) selftest which internally uses
ctr(aes) implemented by the CAAM engine.
Fixes: 115957bb3e59 ("crypto: caam - fix IV DMA mapping and updating")
Signed-off-by: Sascha Hauer <[email protected]>
Cc: [email protected]
---
drivers/crypto/caam/caamalg.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 869f092432de..c05c7938439c 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -917,10 +917,10 @@ static void skcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
{
struct skcipher_request *req = context;
struct skcipher_edesc *edesc;
-#ifdef DEBUG
struct crypto_skcipher *skcipher = crypto_skcipher_reqtfm(req);
int ivsize = crypto_skcipher_ivsize(skcipher);
+#ifdef DEBUG
dev_err(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
#endif
@@ -937,6 +937,14 @@ static void skcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
edesc->dst_nents > 1 ? 100 : req->cryptlen, 1);
skcipher_unmap(jrdev, edesc, req);
+
+ /*
+ * The crypto API expects us to set the IV (req->iv) to the last
+ * ciphertext block.
+ */
+ scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
+ ivsize, 0);
+
kfree(edesc);
skcipher_request_complete(req, err);
@@ -1588,13 +1596,6 @@ static int skcipher_decrypt(struct skcipher_request *req)
if (IS_ERR(edesc))
return PTR_ERR(edesc);
- /*
- * The crypto API expects us to set the IV (req->iv) to the last
- * ciphertext block.
- */
- scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
- ivsize, 0);
-
/* Create and submit job descriptor*/
init_skcipher_job(req, edesc, false);
desc = edesc->hw_desc;
--
2.19.1
Horia,
On Fri, Dec 07, 2018 at 12:31:23PM +0100, Sascha Hauer wrote:
> The crypto API wants the updated IV in req->info after decryption. The
> updated IV used to be copied correctly to req->info after running the
> decryption job. Since 115957bb3e59 this is done before running the job
> so instead of the updated IV only the unmodified input IV is given back
> to the crypto API.
>
> This was observed running the gcm(aes) selftest which internally uses
> ctr(aes) implemented by the CAAM engine.
>
> Fixes: 115957bb3e59 ("crypto: caam - fix IV DMA mapping and updating")
>
> Signed-off-by: Sascha Hauer <[email protected]>
> Cc: [email protected]
> ---
> drivers/crypto/caam/caamalg.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
> index 869f092432de..c05c7938439c 100644
> --- a/drivers/crypto/caam/caamalg.c
> +++ b/drivers/crypto/caam/caamalg.c
> @@ -937,6 +937,14 @@ static void skcipher_decrypt_done(struct device *jrdev, u32 *desc, u32 err,
> edesc->dst_nents > 1 ? 100 : req->cryptlen, 1);
>
> skcipher_unmap(jrdev, edesc, req);
> +
> + /*
> + * The crypto API expects us to set the IV (req->iv) to the last
> + * ciphertext block.
> + */
> + scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
> + ivsize, 0);
> +
I was wrong. It's not adding the scatterwalk_map_and_copy() here which
fixes gcm(aes) selftest. In fact, this has not to be done.
> @@ -1588,13 +1596,6 @@ static int skcipher_decrypt(struct skcipher_request *req)
> if (IS_ERR(edesc))
> return PTR_ERR(edesc);
>
> - /*
> - * The crypto API expects us to set the IV (req->iv) to the last
> - * ciphertext block.
> - */
> - scatterwalk_map_and_copy(req->iv, req->src, req->cryptlen - ivsize,
> - ivsize, 0);
> -
It's the removal of the scatterwalk_map_and_copy() here which fixes
things. With the above the initialization vector which gets passed in is
overwritten.
Now I don't know enough of the crypto stuff to judge if overwriting the IV
always has to be removed or just in some cases, but as a matter of fact
removing these lines fixes the gcm(aes) selftest on i.MX6. From
115957bb3e59 ("crypto: caam - fix IV DMA mapping and updating")
insmodding tcrypt fails with:
alg: aead: decryption failed on test 1 for gcm_base(ctr-aes-caam,ghash-generic): ret=74
alg: aead: Failed to load transform for gcm(aes): -2
alg: aead: Failed to load transform for rfc4106(gcm(aes)): -2
alg: aead: Failed to load transform for rfc4543(gcm(aes)): -2
With the overwriting removed it works again.
Horia, does this make sense to you or is there more that is wrong here?
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |