[PATCH 1/2] crypto: akcipher - use a separate buffer in the sync helpers

[Stephane Debusne]

crypto_akcipher_sync_prep() places the data buffer right after the
request context, in the same allocation, and passes it to the driver
in place (src == dst).

On non-coherent DMA, the start of this buffer may share a cache line
with the end of the request context. A DMA-based driver such as the
CAAM (caampkc) writes req_ctx->edesc and req_ctx->akcipher_op_done
after dma_map_sg() of the destination, which dirties that line. On
arm64, dma_unmap_sg(DMA_FROM_DEVICE) cleans and invalidates partial
lines (dc civac), so the stale CPU copy is written back over the result
written by the device.

crypto_akcipher_sync_encrypt() and crypto_akcipher_sync_decrypt() are
used by rsassa_pkcs1_sign() and by software_key_eds_op() (keyctl
pkey_encrypt and pkey_decrypt), so RSA signing and encryption through
rsa-caam are affected.

On i.MX8M Mini (CAAM era 9, rsa-caam, 6.18), the buffer starts at
offset 40 of a 64-byte line shared with the end of the request context,
and crypto_akcipher_sync_encrypt() returns a result whose first 24
bytes still hold the input in 199 of 200 calls.

Allocate the data buffer separately, so that it is DMA-safe like any
kmalloc() buffer.

Fixes: addde1f2c966 ("crypto: akcipher - Add sync interface without SG lists")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Stephane Debusne <stephane.debusne@xxxxxxxxxx>
---
 crypto/akcipher.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index 630bb1973..2381ed43b 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -170,23 +170,29 @@ static int crypto_akcipher_sync_prep(struct crypto_akcipher_sync_data *data)
 	struct akcipher_request *req;
 	struct scatterlist *sg;
 	unsigned int mlen;
-	unsigned int len;
 	u8 *buf;
 
 	mlen = max(data->slen, data->dlen);
 
-	len = sizeof(*req) + reqsize + mlen;
-	if (len < mlen)
-		return -EOVERFLOW;
-
-	req = kzalloc(len, GFP_KERNEL);
+	req = kzalloc(sizeof(*req) + reqsize, GFP_KERNEL);
 	if (!req)
 		return -ENOMEM;
 
+	/*
+	 * The data buffer is mapped for DMA by hardware drivers. Keep it in
+	 * its own allocation: placed right after the request context, it
+	 * may share a cache line with fields the driver writes while the
+	 * buffer is mapped, which corrupts the result on non-coherent DMA.
+	 */
+	buf = kzalloc(mlen, GFP_KERNEL);
+	if (!buf) {
+		kfree(req);
+		return -ENOMEM;
+	}
+
 	data->req = req;
 	akcipher_request_set_tfm(req, data->tfm);
 
-	buf = (u8 *)(req + 1) + reqsize;
 	data->buf = buf;
 	memcpy(buf, data->src, data->slen);
 
@@ -207,6 +213,7 @@ static int crypto_akcipher_sync_post(struct crypto_akcipher_sync_data *data,
 	err = crypto_wait_req(err, &data->cwait);
 	memcpy(data->dst, data->buf, data->dlen);
 	data->dlen = data->req->dst_len;
+	kfree_sensitive(data->buf);
 	kfree_sensitive(data->req);
 	return err;
 }
-- 
2.43.0