diff mbox series

[cryptodev-2.6] crypto: sig - Fix oops on KEYCTL_PKEY_QUERY for RSA keys

Message ID ff7a28cddfc28e7a3fb8292c680510f35ec54391.1728898147.git.lukas@wunner.de (mailing list archive)
State Accepted
Delegated to: Herbert Xu
Headers show
Series [cryptodev-2.6] crypto: sig - Fix oops on KEYCTL_PKEY_QUERY for RSA keys | expand

Commit Message

Lukas Wunner Oct. 14, 2024, 9:43 a.m. UTC
Commit a2471684dae2 ("crypto: ecdsa - Move X9.62 signature size
calculation into template") introduced ->max_size() and ->digest_size()
callbacks to struct sig_alg.  They return an algorithm's maximum
signature size and digest size, respectively.

For algorithms which lack these callbacks, crypto_register_sig() was
amended to use the ->key_size() callback instead.

However the commit neglected to also amend sig_register_instance().
As a result, the ->max_size() and ->digest_size() callbacks remain NULL
pointers if instances do not define them.  A KEYCTL_PKEY_QUERY system
call results in an oops for such instances:

  BUG: kernel NULL pointer dereference, address: 0000000000000000
  Call Trace:
  software_key_query+0x169/0x370
  query_asymmetric_key+0x67/0x90
  keyctl_pkey_query+0x86/0x120
  __do_sys_keyctl+0x428/0x480
  do_syscall_64+0x4b/0x110

The only instances affected by this are "pkcs1(rsa, ...)".

Fix by moving the callback checks from crypto_register_sig() to
sig_prepare_alg(), which is also invoked by sig_register_instance().
Change the return type of sig_prepare_alg() from void to int to be able
to return errors.  This matches other algorithm types, see e.g.
aead_prepare_alg() or ahash_prepare_alg().

Fixes: a2471684dae2 ("crypto: ecdsa - Move X9.62 signature size calculation into template")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 crypto/sig.c | 36 ++++++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 12 deletions(-)

Comments

Herbert Xu Oct. 26, 2024, 6:53 a.m. UTC | #1
On Mon, Oct 14, 2024 at 11:43:01AM +0200, Lukas Wunner wrote:
> Commit a2471684dae2 ("crypto: ecdsa - Move X9.62 signature size
> calculation into template") introduced ->max_size() and ->digest_size()
> callbacks to struct sig_alg.  They return an algorithm's maximum
> signature size and digest size, respectively.
> 
> For algorithms which lack these callbacks, crypto_register_sig() was
> amended to use the ->key_size() callback instead.
> 
> However the commit neglected to also amend sig_register_instance().
> As a result, the ->max_size() and ->digest_size() callbacks remain NULL
> pointers if instances do not define them.  A KEYCTL_PKEY_QUERY system
> call results in an oops for such instances:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000000
>   Call Trace:
>   software_key_query+0x169/0x370
>   query_asymmetric_key+0x67/0x90
>   keyctl_pkey_query+0x86/0x120
>   __do_sys_keyctl+0x428/0x480
>   do_syscall_64+0x4b/0x110
> 
> The only instances affected by this are "pkcs1(rsa, ...)".
> 
> Fix by moving the callback checks from crypto_register_sig() to
> sig_prepare_alg(), which is also invoked by sig_register_instance().
> Change the return type of sig_prepare_alg() from void to int to be able
> to return errors.  This matches other algorithm types, see e.g.
> aead_prepare_alg() or ahash_prepare_alg().
> 
> Fixes: a2471684dae2 ("crypto: ecdsa - Move X9.62 signature size calculation into template")
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  crypto/sig.c | 36 ++++++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 12 deletions(-)

Patch applied.  Thanks.
diff mbox series

Patch

diff --git a/crypto/sig.c b/crypto/sig.c
index be5ac0e59384..5e1f1f739da2 100644
--- a/crypto/sig.c
+++ b/crypto/sig.c
@@ -84,15 +84,6 @@  struct crypto_sig *crypto_alloc_sig(const char *alg_name, u32 type, u32 mask)
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_sig);
 
-static void sig_prepare_alg(struct sig_alg *alg)
-{
-	struct crypto_alg *base = &alg->base;
-
-	base->cra_type = &crypto_sig_type;
-	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
-	base->cra_flags |= CRYPTO_ALG_TYPE_SIG;
-}
-
 static int sig_default_sign(struct crypto_sig *tfm,
 			    const void *src, unsigned int slen,
 			    void *dst, unsigned int dlen)
@@ -113,7 +104,7 @@  static int sig_default_set_key(struct crypto_sig *tfm,
 	return -ENOSYS;
 }
 
-int crypto_register_sig(struct sig_alg *alg)
+static int sig_prepare_alg(struct sig_alg *alg)
 {
 	struct crypto_alg *base = &alg->base;
 
@@ -132,7 +123,22 @@  int crypto_register_sig(struct sig_alg *alg)
 	if (!alg->digest_size)
 		alg->digest_size = alg->key_size;
 
-	sig_prepare_alg(alg);
+	base->cra_type = &crypto_sig_type;
+	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
+	base->cra_flags |= CRYPTO_ALG_TYPE_SIG;
+
+	return 0;
+}
+
+int crypto_register_sig(struct sig_alg *alg)
+{
+	struct crypto_alg *base = &alg->base;
+	int err;
+
+	err = sig_prepare_alg(alg);
+	if (err)
+		return err;
+
 	return crypto_register_alg(base);
 }
 EXPORT_SYMBOL_GPL(crypto_register_sig);
@@ -146,9 +152,15 @@  EXPORT_SYMBOL_GPL(crypto_unregister_sig);
 int sig_register_instance(struct crypto_template *tmpl,
 			  struct sig_instance *inst)
 {
+	int err;
+
 	if (WARN_ON(!inst->free))
 		return -EINVAL;
-	sig_prepare_alg(&inst->alg);
+
+	err = sig_prepare_alg(&inst->alg);
+	if (err)
+		return err;
+
 	return crypto_register_instance(tmpl, sig_crypto_instance(inst));
 }
 EXPORT_SYMBOL_GPL(sig_register_instance);