diff mbox series

crypto: rsassa-pkcs1 - Copy source data for SG list

Message ID Z0mPDA31r_LEYzNq@gondor.apana.org.au
State Accepted
Commit 8552cb04e0831df3ff265c75ad33f705a45bc731
Headers show
Series crypto: rsassa-pkcs1 - Copy source data for SG list | expand

Commit Message

Herbert Xu Nov. 29, 2024, 9:53 a.m. UTC
As virtual addresses in general may not be suitable for DMA, always
perform a copy before using them in an SG list.

Fixes: 1e562deacecc ("crypto: rsassa-pkcs1 - Migrate to sig_alg backend")
Reported-by: Zorro Lang <zlang@redhat.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

Comments

Lukas Wunner Nov. 30, 2024, 8:41 a.m. UTC | #1
On Fri, Nov 29, 2024 at 05:53:16PM +0800, Herbert Xu wrote:
> As virtual addresses in general may not be suitable for DMA, always
> perform a copy before using them in an SG list.

Just a heads-up, this won't work for use cases when the src buffer
isn't accessible by the kernel.  E.g. if the virtual address pointed to
by src is in TEE restricted memory which was mapped into virtual address
space by dma_buf_vmap():

https://lore.kernel.org/all/20241128150927.1377981-1-jens.wiklander@linaro.org/

Thanks,

Lukas
Herbert Xu Dec. 3, 2024, 7:57 a.m. UTC | #2
On Sat, Nov 30, 2024 at 09:41:40AM +0100, Lukas Wunner wrote:
>
> Just a heads-up, this won't work for use cases when the src buffer
> isn't accessible by the kernel.  E.g. if the virtual address pointed to
> by src is in TEE restricted memory which was mapped into virtual address
> space by dma_buf_vmap():
> 
> https://lore.kernel.org/all/20241128150927.1377981-1-jens.wiklander@linaro.org/

If it's not accessible by the kernel, why would you map into the
kernel page table? That just makes no sense.

Cheers,
diff mbox series

Patch

diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
index 4d077fc96076..f68ffd338f48 100644
--- a/crypto/rsassa-pkcs1.c
+++ b/crypto/rsassa-pkcs1.c
@@ -163,10 +163,6 @@  static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
 	struct rsassa_pkcs1_inst_ctx *ictx = sig_instance_ctx(inst);
 	const struct hash_prefix *hash_prefix = ictx->hash_prefix;
 	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;
-	struct scatterlist in_sg[3], out_sg;
-	struct crypto_wait cwait;
 	unsigned int pad_len;
 	unsigned int ps_end;
 	unsigned int len;
@@ -187,37 +183,25 @@  static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
 
 	pad_len = ctx->key_size - slen - hash_prefix->size - 1;
 
-	child_req = kmalloc(sizeof(*child_req) + child_reqsize + pad_len,
-			    GFP_KERNEL);
-	if (!child_req)
-		return -ENOMEM;
-
 	/* RFC 8017 sec 8.2.1 step 1 - EMSA-PKCS1-v1_5 encoding generation */
-	in_buf = (u8 *)(child_req + 1) + child_reqsize;
+	in_buf = dst;
+	memmove(in_buf + pad_len + hash_prefix->size, src, slen);
+	memcpy(in_buf + pad_len, hash_prefix->data, hash_prefix->size);
+
 	ps_end = pad_len - 1;
 	in_buf[0] = 0x01;
 	memset(in_buf + 1, 0xff, ps_end - 1);
 	in_buf[ps_end] = 0x00;
 
-	/* RFC 8017 sec 8.2.1 step 2 - RSA signature */
-	crypto_init_wait(&cwait);
-	sg_init_table(in_sg, 3);
-	sg_set_buf(&in_sg[0], in_buf, pad_len);
-	sg_set_buf(&in_sg[1], hash_prefix->data, hash_prefix->size);
-	sg_set_buf(&in_sg[2], src, slen);
-	sg_init_one(&out_sg, dst, dlen);
-	akcipher_request_set_tfm(child_req, ctx->child);
-	akcipher_request_set_crypt(child_req, in_sg, &out_sg,
-				   ctx->key_size - 1, dlen);
-	akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
-				      crypto_req_done, &cwait);
 
-	err = crypto_akcipher_decrypt(child_req);
-	err = crypto_wait_req(err, &cwait);
-	if (err)
+	/* RFC 8017 sec 8.2.1 step 2 - RSA signature */
+	err = crypto_akcipher_sync_decrypt(ctx->child, in_buf,
+					   ctx->key_size - 1, in_buf,
+					   ctx->key_size);
+	if (err < 0)
 		return err;
 
-	len = child_req->dst_len;
+	len = err;
 	pad_len = ctx->key_size - len;
 
 	/* Four billion to one */
@@ -239,8 +223,8 @@  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;
-	struct scatterlist in_sg, out_sg;
 	struct crypto_wait cwait;
+	struct scatterlist sg;
 	unsigned int dst_len;
 	unsigned int pos;
 	u8 *out_buf;
@@ -259,13 +243,12 @@  static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
 		return -ENOMEM;
 
 	out_buf = (u8 *)(child_req + 1) + child_reqsize;
+	memcpy(out_buf, src, slen);
 
 	crypto_init_wait(&cwait);
-	sg_init_one(&in_sg, src, slen);
-	sg_init_one(&out_sg, out_buf, ctx->key_size);
+	sg_init_one(&sg, out_buf, slen);
 	akcipher_request_set_tfm(child_req, ctx->child);
-	akcipher_request_set_crypt(child_req, &in_sg, &out_sg,
-				   slen, ctx->key_size);
+	akcipher_request_set_crypt(child_req, &sg, &sg, slen, slen);
 	akcipher_request_set_callback(child_req, CRYPTO_TFM_REQ_MAY_SLEEP,
 				      crypto_req_done, &cwait);