Zorro reports a BUG_ON() when running boot-time crypto selftests on
arm64:
Since commit 1e562deacecc ("crypto: rsassa-pkcs1 - Migrate to sig_alg
backend"), sg_set_buf() is called with the address of a test vector in
the .rodata section. That fails on arm64 as virt_addr_valid() returns
false. Architectures such as x86 are more lenient and transparently
translate the virtual address pointing into the kernel image to a
physical address. However Mark points out that the BUG_ON() may still
occur if testmgr.c is built as a module because the test vectors would
then be module/vmalloc addresses rather than virt/linear or kernel image
addresses.
Avoid the issue by auto-detecting whether the src buffer of a sign or
verify operation is a valid virtual address and duplicating the buffer
if not.
An alternative approach would be to use crypto_akcipher_sync_encrypt()
and crypto_akcipher_sync_decrypt(), however that would *always*
duplicate the src buffer (rather than only when it's necessary)
and thus negatively impact performance.
In addition to the src buffer, the Full Hash Prefix likewise lives in
the .rodata section and is always included in a scatterlist when
performing a sign operation, so duplicate it on instance creation.
Fixes: 1e562deacecc ("crypto: rsassa-pkcs1 - Migrate to sig_alg backend")
Reported-by: Zorro Lang <zlang@xxxxxxxxxx>
Closes: https://lore.kernel.org/r/20241122045106.tzhvm2wrqvttub6k@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Cc: Mark Rutland <mark.rutland@xxxxxxx>
---
crypto/rsassa-pkcs1.c | 33 ++++++++++++++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/crypto/rsassa-pkcs1.c b/crypto/rsassa-pkcs1.c
index 4d077fc9..5839a85 100644
--- a/crypto/rsassa-pkcs1.c
+++ b/crypto/rsassa-pkcs1.c
@@ -153,6 +153,7 @@ struct rsassa_pkcs1_ctx {
struct rsassa_pkcs1_inst_ctx {
struct crypto_akcipher_spawn spawn;
const struct hash_prefix *hash_prefix;
+ const u8 *hash_prefix_data;
};
static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
@@ -165,6 +166,7 @@ static int rsassa_pkcs1_sign(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;
+ void *src_copy __free(kfree_sensitive) = NULL;
struct scatterlist in_sg[3], out_sg;
struct crypto_wait cwait;
unsigned int pad_len;
@@ -185,6 +187,16 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
if (slen + hash_prefix->size > ctx->key_size - 11)
return -EOVERFLOW;
+ /*
+ * Only kmalloc virtual addresses shall be used in a scatterlist,
+ * so duplicate src if it points e.g. into kernel or module rodata.
+ */
+ if (!virt_addr_valid(src)) {
+ src = src_copy = kmemdup(src, slen, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ }
+
pad_len = ctx->key_size - slen - hash_prefix->size - 1;
child_req = kmalloc(sizeof(*child_req) + child_reqsize + pad_len,
@@ -203,7 +215,7 @@ static int rsassa_pkcs1_sign(struct crypto_sig *tfm,
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[1], ictx->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);
@@ -239,6 +251,7 @@ 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;
+ void *src_copy __free(kfree_sensitive) = NULL;
struct scatterlist in_sg, out_sg;
struct crypto_wait cwait;
unsigned int dst_len;
@@ -252,6 +265,16 @@ static int rsassa_pkcs1_verify(struct crypto_sig *tfm,
rsassa_pkcs1_invalid_hash_len(dlen, hash_prefix))
return -EINVAL;
+ /*
+ * Only kmalloc virtual addresses shall be used in a scatterlist,
+ * so duplicate src if it points e.g. into kernel or module rodata.
+ */
+ if (!virt_addr_valid(src)) {
+ src = src_copy = kmemdup(src, slen, GFP_KERNEL);
+ if (!src)
+ return -ENOMEM;
+ }
+
/* RFC 8017 sec 8.2.2 step 2 - RSA verification */
child_req = kmalloc(sizeof(*child_req) + child_reqsize + ctx->key_size,
GFP_KERNEL);
@@ -366,6 +389,7 @@ static void rsassa_pkcs1_free(struct sig_instance *inst)
struct crypto_akcipher_spawn *spawn = &ctx->spawn;
crypto_drop_akcipher(spawn);
+ kfree(ctx->hash_prefix_data);
kfree(inst);
}
@@ -412,6 +436,13 @@ static int rsassa_pkcs1_create(struct crypto_template *tmpl, struct rtattr **tb)
goto err_free_inst;
}
+ ctx->hash_prefix_data = kmemdup(ctx->hash_prefix->data,
+ ctx->hash_prefix->size, GFP_KERNEL);
+ if (!ctx->hash_prefix_data) {
+ err = -ENOMEM;
+ goto err_free_inst;
+ }
+
err = -ENAMETOOLONG;
if (snprintf(inst->alg.base.cra_name, CRYPTO_MAX_ALG_NAME,
"pkcs1(%s,%s)", rsa_alg->base.cra_name,
--
2.43.0
