On 2/5/2019 10:31 AM, Sascha Hauer wrote:
Hi Horia,
On Mon, Feb 04, 2019 at 12:26:26PM +0000, Horia Geanta wrote:
On 1/31/2019 8:12 AM, Sascha Hauer wrote:
In skcipher_decrypt() the IV passed in by the caller is overwritten and the tcrypt module 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
With this patch tcrypt runs without errors.
This doesn't mean the patch is correct. crypto API requires skcipher implementations to update the IV with the last ciphertext block.
The root cause of the issue is cache line sharing.
struct crypto_gcm_req_priv_ctx { u8 iv[16]; u8 auth_tag[16]; [...] };
Since caam does not support ghash on i.MX6, only ctr skcipher part of the gcm is offloaded. The skcipher request received by caam has req->src pointing to auth_tag[16] (1st S/G entry) and req->iv pointing to iv[16]. caam driver: 1-DMA maps req->src 2-copies original req->iv to internal buffer 3-updates req->iv (scatterwalk_map_and_copy from last block in req->src) 4-sends job to crypto engine
Problem is that operation 3 above is writing iv[16], which is on the same cache line as auth_tag[16] that was previously DMA mapped.
I've checked that forcing auth_tag and iv to be on separate cache lines
u8 auth_tag[16];
u8 auth_tag[16] ____cacheline_aligned;
solves the issue.
I can confirm that this solves it here on my side aswell.
Thanks for confirming.
I have no idea what's best to do here, but I'd like to have that fixed.
Yes, me too. I would wait for Herbert's input though. As I said, it's a bit weird for a skcipher implementation to be aware of updating req->iv having such a side effect on req->src.
Is there some easy to reproduce testcase to show the issues that arise with my patch? Apparently tcrypt doesn't do chaining, right?
In theory cts(cbc(aes)) decryption would have to fail when cbc(aes) would execute on caam and cts() in SW.
However, cts() SW implementation (crypto/cts.c) does not rely on underlying cbc(aes) to update the IV, instead it uses a temporary saved IV - see discussion here: https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg27719.html
Horia