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 <s.ha...@pengutronix.de>
> Cc: sta...@vger.kernel.org
> ---
> 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 |
