Crypto: Fix one bug when use kernel ecdsa algorithm

[PATCH] Crypto: Fix one bug when use kernel ecdsa algorithm

Posted by yangmengfei1394@phytium.com.cn 2 years, 9 months ago

From: ymf <yangmengfei1394@phytium.com.cn>

We are trying to use kernel ecdsa algorithm to sign something.
When we call the crypto_akcipher_set_priv_key function, the
system collapses because it comes to NULL pointer.We find out
kerenl ecdsa algorithm has not offered the set_priv_key function.
So we think it might be necessary to check whether set_priv_key is
available when crypto_akcipher_set_priv_key is called.

Signed-off-by: ymf <yangmengfei1394@phytium.com.cn>
---
 include/crypto/akcipher.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
index f35fd653e4e5..a68f0e23bf89 100644
--- a/include/crypto/akcipher.h
+++ b/include/crypto/akcipher.h
@@ -459,7 +459,9 @@ static inline int crypto_akcipher_set_priv_key(struct crypto_akcipher *tfm,
 					       unsigned int keylen)
 {
 	struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
-
-	return alg->set_priv_key(tfm, key, keylen);
+	if (likely(alg->set_priv_key != NULL))
+		return alg->set_priv_key(tfm, key, keylen);
+	else
+		return -EPERM;
 }
 #endif
-- 
2.34.1

Re: [PATCH] Crypto: Fix one bug when use kernel ecdsa algorithm

Posted by Herbert Xu 2 years, 8 months ago

yangmengfei1394@phytium.com.cn wrote:
>
> diff --git a/include/crypto/akcipher.h b/include/crypto/akcipher.h
> index f35fd653e4e5..a68f0e23bf89 100644
> --- a/include/crypto/akcipher.h
> +++ b/include/crypto/akcipher.h
> @@ -459,7 +459,9 @@ static inline int crypto_akcipher_set_priv_key(struct crypto_akcipher *tfm,
>                                               unsigned int keylen)
> {
>        struct akcipher_alg *alg = crypto_akcipher_alg(tfm);
> -
> -       return alg->set_priv_key(tfm, key, keylen);
> +       if (likely(alg->set_priv_key != NULL))
> +               return alg->set_priv_key(tfm, key, keylen);
> +       else
> +               return -EPERM;
> }
> #endif

Instead of doing this, we should move the code that sets the
default functions from crypto_register_akcipher into
akcipher_prepare_alg.

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

[PATCH] Crypto module : rearrange the default functions of akcipher

Posted by yangmengfei1394 2 years, 8 months ago

According to your last email, we rearrange the default functions,
move the code that sets the default functions from
crypto_register_akcipher into akcipher_prepare_alg,
add the set_pub_key function check at the same time.

Signed-off-by: yangmengfei1394 <yangmengfei1394@phytium.com.cn>
---
 crypto/akcipher.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/crypto/akcipher.c b/crypto/akcipher.c
index 7960ceb528c3..45502446b231 100644
--- a/crypto/akcipher.c
+++ b/crypto/akcipher.c
@@ -126,19 +126,6 @@ struct crypto_akcipher *crypto_alloc_akcipher(const char *alg_name, u32 type,
 }
 EXPORT_SYMBOL_GPL(crypto_alloc_akcipher);
 
-static void akcipher_prepare_alg(struct akcipher_alg *alg)
-{
-	struct crypto_istat_akcipher *istat = akcipher_get_stat(alg);
-	struct crypto_alg *base = &alg->base;
-
-	base->cra_type = &crypto_akcipher_type;
-	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
-	base->cra_flags |= CRYPTO_ALG_TYPE_AKCIPHER;
-
-	if (IS_ENABLED(CONFIG_CRYPTO_STATS))
-		memset(istat, 0, sizeof(*istat));
-}
-
 static int akcipher_default_op(struct akcipher_request *req)
 {
 	return -ENOSYS;
@@ -150,8 +137,9 @@ static int akcipher_default_set_key(struct crypto_akcipher *tfm,
 	return -ENOSYS;
 }
 
-int crypto_register_akcipher(struct akcipher_alg *alg)
+static void akcipher_prepare_alg(struct akcipher_alg *alg)
 {
+	struct crypto_istat_akcipher *istat = akcipher_get_stat(alg);
 	struct crypto_alg *base = &alg->base;
 
 	if (!alg->sign)
@@ -164,6 +152,20 @@ int crypto_register_akcipher(struct akcipher_alg *alg)
 		alg->decrypt = akcipher_default_op;
 	if (!alg->set_priv_key)
 		alg->set_priv_key = akcipher_default_set_key;
+	if (!alg->set_pub_key)
+		alg->set_pub_key = akcipher_default_set_key;
+
+	base->cra_type = &crypto_akcipher_type;
+	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
+	base->cra_flags |= CRYPTO_ALG_TYPE_AKCIPHER;
+
+	if (IS_ENABLED(CONFIG_CRYPTO_STATS))
+		memset(istat, 0, sizeof(*istat));
+}
+
+int crypto_register_akcipher(struct akcipher_alg *alg)
+{
+	struct crypto_alg *base = &alg->base;
 
 	akcipher_prepare_alg(alg);
 	return crypto_register_alg(base);
-- 
2.35.1.windows.2

Re: [PATCH] Crypto module : rearrange the default functions of akcipher

Posted by Herbert Xu 2 years, 8 months ago

On Mon, May 22, 2023 at 11:30:27AM +0800, yangmengfei1394 wrote:
> According to your last email, we rearrange the default functions,
> move the code that sets the default functions from
> crypto_register_akcipher into akcipher_prepare_alg,
> add the set_pub_key function check at the same time.

Why are you adding set_pub_key? Every implementation should
provide this function.

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