| Message ID | 1616739212-7751-1-git-send-email-yumeng18@huawei.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: hisilicon/hpre - rsa key should not be empty | expand |
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,
在 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.
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,
在 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 --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;
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(+)