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>

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);
 
-- 
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

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

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,
-- 
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