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

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

[Meng Yu]

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

Re: [PATCH] crypto: hisilicon/hpre - rsa key should not be empty

[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?

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

Re: [PATCH] crypto: hisilicon/hpre - rsa key should not be empty

[yumeng]

在 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.

Re: [PATCH] crypto: hisilicon/hpre - rsa key should not be empty

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

Re: [PATCH] crypto: hisilicon/hpre - rsa key should not be empty

[yumeng]

在 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.