diff mbox series

crypto: hisilicon/hpre - rsa key should not be empty

Message ID 1616739212-7751-1-git-send-email-yumeng18@huawei.com
State New
Headers show
Series crypto: hisilicon/hpre - rsa key should not be empty | expand

Commit Message

yumeng March 26, 2021, 6:13 a.m. UTC
We should ensure key is not empty before we set key.

Signed-off-by: Meng Yu <yumeng18@huawei.com>
---
 drivers/crypto/hisilicon/hpre/hpre_crypto.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Herbert Xu April 2, 2021, 7:12 a.m. UTC | #1
On Fri, Mar 26, 2021 at 02:13:32PM +0800, Meng Yu wrote:
> We should ensure key is not empty before we set key.

> 

> Signed-off-by: Meng Yu <yumeng18@huawei.com>

> ---

>  drivers/crypto/hisilicon/hpre/hpre_crypto.c | 6 ++++++

>  1 file changed, 6 insertions(+)

> 

> diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c

> index 53068d2..7cf7d80 100644

> --- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c

> +++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c

> @@ -1093,6 +1093,9 @@ static int hpre_rsa_setpubkey(struct crypto_akcipher *tfm, const void *key,

>  	struct hpre_ctx *ctx = akcipher_tfm_ctx(tfm);

>  	int ret;

>  

> +	if (!key || !keylen)

> +		return -EINVAL;

> +

>  	ret = crypto_akcipher_set_pub_key(ctx->rsa.soft_tfm, key, keylen);


Does this fix a real bug? Wouldn't the soft fallback setkey catch
this anyhow?

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
yumeng April 2, 2021, 10:16 a.m. UTC | #2
在 2021/4/2 15:12, Herbert Xu 写道:
> On Fri, Mar 26, 2021 at 02:13:32PM +0800, Meng Yu wrote:

>> We should ensure key is not empty before we set key.

>>

>> Signed-off-by: Meng Yu <yumeng18@huawei.com>

>> ---

>>   drivers/crypto/hisilicon/hpre/hpre_crypto.c | 6 ++++++

>>   1 file changed, 6 insertions(+)

>>

>> diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c

>> index 53068d2..7cf7d80 100644

>> --- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c

>> +++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c

>> @@ -1093,6 +1093,9 @@ static int hpre_rsa_setpubkey(struct crypto_akcipher *tfm, const void *key,

>>   	struct hpre_ctx *ctx = akcipher_tfm_ctx(tfm);

>>   	int ret;

>>   

>> +	if (!key || !keylen)

>> +		return -EINVAL;

>> +

>>   	ret = crypto_akcipher_set_pub_key(ctx->rsa.soft_tfm, key, keylen);

> 

> Does this fix a real bug? Wouldn't the soft fallback setkey catch

> this anyhow?

> 


I think it is not a real bug, and soft fallback setkey can always catch 
the error.
But our original intention was to make it don't go to 'xxx_set_pub_key'
when the key is null, and it can return an error earlier.
But maybe it is not good.
Herbert Xu April 2, 2021, 10:22 a.m. UTC | #3
On Fri, Apr 02, 2021 at 06:16:16PM +0800, yumeng wrote:
>

> I think it is not a real bug, and soft fallback setkey can always catch the

> error.

> But our original intention was to make it don't go to 'xxx_set_pub_key'

> when the key is null, and it can return an error earlier.

> But maybe it is not good.


It might make sense to check them twice if you were touching them
directly, e.g., poking inside the key.  However, it appears that
your driver simply palms off the key to rsa_helper.c which should
check the key/keylen too so I think there is no need for this patch
for now.

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
yumeng April 6, 2021, 12:51 a.m. UTC | #4
在 2021/4/2 18:22, Herbert Xu 写道:
> On Fri, Apr 02, 2021 at 06:16:16PM +0800, yumeng wrote:

>>

>> I think it is not a real bug, and soft fallback setkey can always catch the

>> error.

>> But our original intention was to make it don't go to 'xxx_set_pub_key'

>> when the key is null, and it can return an error earlier.

>> But maybe it is not good.

> 

> It might make sense to check them twice if you were touching them

> directly, e.g., poking inside the key.  However, it appears that

> your driver simply palms off the key to rsa_helper.c which should

> check the key/keylen too so I think there is no need for this patch

> for now.

> 

> Thanks,

> 


OK, thank you.
diff mbox series

Patch

diff --git a/drivers/crypto/hisilicon/hpre/hpre_crypto.c b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
index 53068d2..7cf7d80 100644
--- a/drivers/crypto/hisilicon/hpre/hpre_crypto.c
+++ b/drivers/crypto/hisilicon/hpre/hpre_crypto.c
@@ -1093,6 +1093,9 @@  static int hpre_rsa_setpubkey(struct crypto_akcipher *tfm, const void *key,
 	struct hpre_ctx *ctx = akcipher_tfm_ctx(tfm);
 	int ret;
 
+	if (!key || !keylen)
+		return -EINVAL;
+
 	ret = crypto_akcipher_set_pub_key(ctx->rsa.soft_tfm, key, keylen);
 	if (ret)
 		return ret;
@@ -1106,6 +1109,9 @@  static int hpre_rsa_setprivkey(struct crypto_akcipher *tfm, const void *key,
 	struct hpre_ctx *ctx = akcipher_tfm_ctx(tfm);
 	int ret;
 
+	if (!key || !keylen)
+		return -EINVAL;
+
 	ret = crypto_akcipher_set_priv_key(ctx->rsa.soft_tfm, key, keylen);
 	if (ret)
 		return ret;