[PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver

Changwei Zou posted 1 patch 2 months ago
crypto/rsassa-pkcs1.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
[PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver
Posted by Changwei Zou 2 months ago
out_buf is used as a DMA buffer for the RSA verification operation.
If it is not aligned to CRYPTO_DMA_ALIGN, cacheline sharing
problems (data corruption) would occur on CPUs with DMA-incoherent caches,
leading to -EKEYREJECTED.

Rename out_buf to buf, as it serves as both the input and output buffer.
Add a buf_ptr pointer to track its position.

Allocate the buffer separately via kmalloc(), which guarantees cacheline
alignment on architectures without fully coherent DMA.
This acts as a defensive measure, and avoids the need for an extra copy
in the underlying driver, which should check alignment before supplying
buffers to the hardware.

The intermittent error 'Key was rejected by service' on i.MX8 with CAAM
can be triggered when loading signed kernel modules.

    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

Signed-off-by: Changwei Zou <changwei.zou@canonical.com>
---
 crypto/rsassa-pkcs1.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
index 94fa5e9600e7..b1fb5111b6af 100644
--- a/crypto/rsassa-pkcs1.c
+++ b/crypto/rsassa-pkcs1.c
@@ -223,11 +223,12 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 	struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm);
 	unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child);
 	struct akcipher_request *child_req __free(kfree_sensitive) = NULL;
+	u8 *buf __free(kfree_sensitive) = NULL;
 	struct crypto_wait cwait;
 	struct scatterlist sg;
 	unsigned int dst_len;
 	unsigned int pos;
-	u8 *out_buf;
+	u8 *buf_ptr;
 	int err;
 
 	/* RFC 8017 sec 8.2.2 step 1 - length checking */
@@ -237,16 +238,16 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 		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);
-	if (!child_req)
+	child_req = kmalloc(sizeof(*child_req) + child_reqsize, GFP_KERNEL);
+	buf = kmalloc(ctx->key_size, GFP_KERNEL);
+	if (!child_req || !buf)
 		return -ENOMEM;
 
-	out_buf = (u8 *)(child_req + 1) + child_reqsize;
-	memcpy(out_buf, src, slen);
+	buf_ptr = buf;
+	memcpy(buf_ptr, src, slen);
 
 	crypto_init_wait(&cwait);
-	sg_init_one(&sg, out_buf, slen);
+	sg_init_one(&sg, buf_ptr, slen);
 	akcipher_request_set_tfm(child_req, ctx->child);
 	akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);
 	akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
@@ -263,35 +264,35 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 		return -EINVAL;
 
 	if (dst_len == ctx->key_size) {
-		if (out_buf[0] != 0x00)
+		if (buf_ptr[0] != 0x00)
 			/* Encrypted value had no leading 0 byte */
 			return -EINVAL;
 
 		dst_len--;
-		out_buf++;
+		buf_ptr++;
 	}
 
-	if (out_buf[0] != 0x01)
+	if (buf_ptr[0] != 0x01)
 		return -EBADMSG;
 
 	for (pos = 1; pos < dst_len; pos++)
-		if (out_buf[pos] != 0xff)
+		if (buf_ptr[pos] != 0xff)
 			break;
 
-	if (pos < 9 || pos == dst_len || out_buf[pos] != 0x00)
+	if (pos < 9 || pos == dst_len || buf_ptr[pos] != 0x00)
 		return -EBADMSG;
 	pos++;
 
 	if (hash_prefix->size > dst_len - pos)
 		return -EBADMSG;
-	if (crypto_memneq(out_buf + pos, hash_prefix->data, hash_prefix->size))
+	if (crypto_memneq(buf_ptr + pos, hash_prefix->data, hash_prefix->size))
 		return -EBADMSG;
 	pos += hash_prefix->size;
 
-	/* RFC 8017 sec 8.2.2 step 4 - comparison of digest with out_buf */
+	/* RFC 8017 sec 8.2.2 step 4 - comparison of digest with buf */
 	if (dlen != dst_len - pos)
 		return -EKEYREJECTED;
-	if (memcmp(digest, out_buf + pos, dlen) != 0)
+	if (memcmp(digest, buf_ptr + pos, dlen) != 0)
 		return -EKEYREJECTED;
 
 	return 0;
-- 
2.43.0
Re: [PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver
Posted by Kepplinger-Novakovic Martin 2 days, 23 hours ago
Am Freitag, dem 31.07.2026 um 12:44 +1000 schrieb Changwei Zou:
> out_buf is used as a DMA buffer for the RSA verification operation.
> If it is not aligned to CRYPTO_DMA_ALIGN, cacheline sharing
> problems (data corruption) would occur on CPUs with DMA-incoherent caches,
> leading to -EKEYREJECTED.
> 
> Rename out_buf to buf, as it serves as both the input and output buffer.
> Add a buf_ptr pointer to track its position.
> 
> Allocate the buffer separately via kmalloc(), which guarantees cacheline
> alignment on architectures without fully coherent DMA.
> This acts as a defensive measure, and avoids the need for an extra copy
> in the underlying driver, which should check alignment before supplying
> buffers to the hardware.
> 
> The intermittent error 'Key was rejected by service' on i.MX8 with CAAM
> can be triggered when loading signed kernel modules.
> 
>     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
> 
> Signed-off-by: Changwei Zou <changwei.zou@canonical.com>
> ---
>  crypto/rsassa-pkcs1.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
> index 94fa5e9600e7..b1fb5111b6af 100644
> --- a/crypto/rsassa-pkcs1.c
> +++ b/crypto/rsassa-pkcs1.c
> @@ -223,11 +223,12 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
>  	struct rsassa_pkcs1_ctx *ctx = crypto_sig_ctx(tfm);
>  	unsigned int child_reqsize = crypto_akcipher_reqsize(ctx->child);
>  	struct akcipher_request *child_req __free(kfree_sensitive) = NULL;
> +	u8 *buf __free(kfree_sensitive) = NULL;
>  	struct crypto_wait cwait;
>  	struct scatterlist sg;
>  	unsigned int dst_len;
>  	unsigned int pos;
> -	u8 *out_buf;
> +	u8 *buf_ptr;
>  	int err;
>  
>  	/* RFC 8017 sec 8.2.2 step 1 - length checking */
> @@ -237,16 +238,16 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
>  		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);
> -	if (!child_req)
> +	child_req = kmalloc(sizeof(*child_req) + child_reqsize, GFP_KERNEL);
> +	buf = kmalloc(ctx->key_size, GFP_KERNEL);
> +	if (!child_req || !buf)
>  		return -ENOMEM;
>  
> -	out_buf = (u8 *)(child_req + 1) + child_reqsize;
> -	memcpy(out_buf, src, slen);
> +	buf_ptr = buf;
> +	memcpy(buf_ptr, src, slen);
>  
>  	crypto_init_wait(&cwait);
> -	sg_init_one(&sg, out_buf, slen);
> +	sg_init_one(&sg, buf_ptr, slen);
>  	akcipher_request_set_tfm(child_req, ctx->child);
>  	akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);
>  	akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
> @@ -263,35 +264,35 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
>  		return -EINVAL;
>  
>  	if (dst_len == ctx->key_size) {
> -		if (out_buf[0] != 0x00)
> +		if (buf_ptr[0] != 0x00)
>  			/* Encrypted value had no leading 0 byte */
>  			return -EINVAL;
>  
>  		dst_len--;
> -		out_buf++;
> +		buf_ptr++;
>  	}
>  
> -	if (out_buf[0] != 0x01)
> +	if (buf_ptr[0] != 0x01)
>  		return -EBADMSG;
>  
>  	for (pos = 1; pos < dst_len; pos++)
> -		if (out_buf[pos] != 0xff)
> +		if (buf_ptr[pos] != 0xff)
>  			break;
>  
> -	if (pos < 9 || pos == dst_len || out_buf[pos] != 0x00)
> +	if (pos < 9 || pos == dst_len || buf_ptr[pos] != 0x00)
>  		return -EBADMSG;
>  	pos++;
>  
>  	if (hash_prefix->size > dst_len - pos)
>  		return -EBADMSG;
> -	if (crypto_memneq(out_buf + pos, hash_prefix->data, hash_prefix->size))
> +	if (crypto_memneq(buf_ptr + pos, hash_prefix->data, hash_prefix->size))
>  		return -EBADMSG;
>  	pos += hash_prefix->size;
>  
> -	/* RFC 8017 sec 8.2.2 step 4 - comparison of digest with out_buf */
> +	/* RFC 8017 sec 8.2.2 step 4 - comparison of digest with buf */
>  	if (dlen != dst_len - pos)
>  		return -EKEYREJECTED;
> -	if (memcmp(digest, out_buf + pos, dlen) != 0)
> +	if (memcmp(digest, buf_ptr + pos, dlen) != 0)
>  		return -EKEYREJECTED;
>  
>  	return 0;


Hi Changwei,

Very late, sorry, and maybe this patch is irrelevant(?) but I tested it now. veritysetup() succeeds just
like when using your driver-level patch I tested earlier today:
https://lore.kernel.org/linux-crypto/20260821045555.806471-1-changwei.zou@canonical.com/T/#t

I should have added the following tags to the above email as well:

Reported-by: Martin Kepplinger-Novaković <Martin.Kepplinger-Novakovic@ginzinger.com>
Closes: https://lore.kernel.org/r/6029acc0f0ddfe25e2537c2866d54fd7f54bc182.camel@ginzinger.com

I hope you can come to a conclusion on this!

thanks for working on this,

                              martin
Re: [PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver
Posted by Herbert Xu 2 months ago
On Fri, Jul 31, 2026 at 12:44:46PM +1000, Changwei Zou wrote:
> out_buf is used as a DMA buffer for the RSA verification operation.
> If it is not aligned to CRYPTO_DMA_ALIGN, cacheline sharing
> problems (data corruption) would occur on CPUs with DMA-incoherent caches,
> leading to -EKEYREJECTED.
> 
> Rename out_buf to buf, as it serves as both the input and output buffer.
> Add a buf_ptr pointer to track its position.
> 
> Allocate the buffer separately via kmalloc(), which guarantees cacheline
> alignment on architectures without fully coherent DMA.
> This acts as a defensive measure, and avoids the need for an extra copy
> in the underlying driver, which should check alignment before supplying
> buffers to the hardware.
> 
> The intermittent error 'Key was rejected by service' on i.MX8 with CAAM
> can be triggered when loading signed kernel modules.
> 
>     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
> 
> Signed-off-by: Changwei Zou <changwei.zou@canonical.com>
> ---
>  crypto/rsassa-pkcs1.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)

In the Crypto API we don't ask the user to provide aligned data.

In general it's the driver's responsibility to ensure proper
alignment.  Which driver are we talking about here?

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver
Posted by Changwei Zou 1 month, 4 weeks ago
Hi Herbert,

An orthogonal driver-level patch can be found at the following link.

https://lore.kernel.org/all/20260805041902.1575170-1-changwei.zou@canonical.com/T/#u

Thanks a lot.

Kind regards,
Changwei
Re: [PATCH v5] crypto: rsassa-pkcs1 - Avoid cacheline sharing with underlying driver
Posted by Changwei Zou 2 months ago
Hi Herbert,

Please see the CAAM driver shown below.
Thank you very much for your guidance and support.

	(gdb) bt
	#0  rsa_edesc_alloc () at drivers/crypto/caam/caampkc.c:294
	#1  caam_rsa_enc () at drivers/crypto/caam/caampkc.c:733
	#2  crypto_akcipher_encrypt () at include/crypto/akcipher.h:279
	#3  rsassa_pkcs1_verify () at crypto/rsassa-pkcs1.c:266

Kind regards,
Changwei