[PATCH] crypto: rsassa-pkcs1: fix in-place DMA aliasing in rsassa_pkcs1_verify()

Changwei Zou posted 1 patch 13 hours ago
crypto/rsassa-pkcs1.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
[PATCH] crypto: rsassa-pkcs1: fix in-place DMA aliasing in rsassa_pkcs1_verify()
Posted by Changwei Zou 13 hours ago
rsassa_pkcs1_verify() passes the same scatterlist as both src and dst
to akcipher_request_set_crypt():

    sg_init_one(&sg, out_buf, slen);
    akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);

This causes the underlying hardware driver to DMA-map the same
physical buffer twice with incompatible directions (DMA_TO_DEVICE and
DMA_FROM_DEVICE), which is undefined behaviour on non-coherent
architectures and leads to intermittent cache-coherency failures.

    dma_map_sg(dev, fixup_src, src_nents, DMA_TO_DEVICE);
    dma_map_sg(dev, req->dst,  dst_nents, DMA_FROM_DEVICE);

On non-coherent ARM64 platforms (e.g. i.MX8 with CAAM), the
DMA_FROM_DEVICE mapping invalidates CPU cache lines covering the buffer
after the DMA_TO_DEVICE mapping has flushed them but before the hardware
reads the data. This produces a race that intermittently corrupts the
RSA input, causing the EMSA-PKCS1-v1_5 structure check or digest
comparison to fail with -EKEYREJECTED.

Fix by allocating separate input and output buffers so each DMA mapping
covers a distinct physical region, matching the approach used by the
predecessor pkcs1pad template.

The intermittent error 'Key was rejected by service' on i.MX8 with CAAM
can be triggered when running the command below.

    for i in $(seq 1 100); do
        sudo modprobe xfs 2>&1 && echo "SUCCESS on attempt $i" \
        && sudo rmmod xfs || echo "FAILED on attempt $i"
    done

Fixes: 8552cb04e083 ("crypto: rsassa-pkcs1 - Copy source data for SG list")
Signed-off-by: Changwei Zou <changwei.zou@canonical.com>
---
 crypto/rsassa-pkcs1.c | 29 ++++++++++++++++++++---------
 1 file changed, 20 insertions(+), 9 deletions(-)

diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
index 94fa5e9600e7..63bce31ac446 100644
--- a/crypto/rsassa-pkcs1.c
+++ b/crypto/rsassa-pkcs1.c
@@ -224,10 +224,10 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 	unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child);
 	struct akcipher_request *child_req __free(kfree_sensitive) = NULL;
 	struct crypto_wait cwait;
-	struct scatterlist sg;
+	struct scatterlist sg_src, sg_dst;
 	unsigned int dst_len;
 	unsigned int pos;
-	u8 *out_buf;
+	u8 *in_buf, *out_buf;
 	int err;
 
 	/* RFC 8017 sec 8.2.2 step 1 - length checking */
@@ -236,19 +236,30 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 	    rsassa_pkcs1_invalid_hash_len(dlen, hash_prefix))
 		return -EINVAL;
 
-	/* RFC 8017 sec 8.2.2 step 2 - RSA verification */
-	child_req = kmalloc(sizeof(*child_req) + child_reqsize + ctx->key_size,
-			    GFP_KERNEL);
+	/*
+	 * RFC 8017 sec 8.2.2 step 2 - RSA verification
+	 *
+	 * Allocate separate input and output buffers within the child_req
+	 * allocation.  Using the same buffer for both src and dst (in-place)
+	 * would cause it to be DMA-mapped twice with incompatible directions
+	 * (DMA_TO_DEVICE and DMA_FROM_DEVICE), which is undefined behaviour on
+	 * non-coherent architectures such as ARM64 with hardware accelerators
+	 * like CAAM, leading to intermittent cache-coherency failures.
+	 */
+	child_req = kmalloc(sizeof(*child_req) + child_reqsize +
+			    2 * ctx->key_size, GFP_KERNEL);
 	if (!child_req)
 		return -ENOMEM;
 
-	out_buf = (u8 *)(child_req + 1) + child_reqsize;
-	memcpy(out_buf, src, slen);
+	in_buf  = (u8 *)(child_req + 1) + child_reqsize;
+	out_buf = in_buf + ctx->key_size;
+	memcpy(in_buf, src, slen);
 
 	crypto_init_wait(&cwait);
-	sg_init_one(&sg, out_buf, slen);
+	sg_init_one(&sg_src, in_buf,  slen);
+	sg_init_one(&sg_dst, out_buf, slen);
 	akcipher_request_set_tfm(child_req, ctx->child);
-	akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);
+	akcipher_request_set_crypt(child_req, &sg_src, &sg_dst, slen, slen);
 	akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
 				      crypto_req_done, &cwait);
 
-- 
2.43.0