Re: [PATCH 4/4] crypto: hisilicon/zip - support new 'sqe' type in Kunpeng930
On 2021/3/26 17:14, Herbert Xu wrote:
On Fri, Mar 19, 2021 at 03:33:07PM +0800, Yang Shen wrote:
+const struct hisi_zip_sqe_ops hisi_zip_ops_v2 = {
+ .sqe_type = 0x3,
+ .fill_addr = hisi_zip_fill_addr,
+ .fill_buf_size = hisi_zip_fill_buf_size,
+ .fill_buf_type = hisi_zip_fill_buf_type,
+ .fill_req_type = hisi_zip_fill_req_type,
+ .fill_tag = hisi_zip_fill_tag_v2,
+ .fill_sqe_type = hisi_zip_fill_sqe_type,
+ .get_tag= hisi_zip_get_tag_v2,
+ .get_status = hisi_zip_get_status,
+ .get_dstlen = hisi_zip_get_dstlen,
+};
+
This triggers a new warning:
CHECK ../drivers/crypto/hisilicon/zip/zip_crypto.c
../drivers/crypto/hisilicon/zip/zip_crypto.c:527:31: warning: symbol
'hisi_zip_ops_v1' was not declared. Should it be static?
../drivers/crypto/hisilicon/zip/zip_crypto.c:540:31: warning: symbol
'hisi_zip_ops_v2' was not declared. Should it be static?
Please fix. Thanks.
Sorry， I'll fix this in next version.
Thanks.
Re: [PATCH RESEND 04/10] crypto: hisilicon/zip - replace 'sprintf' with 'scnprintf'
On 2020/9/4 15:40, Herbert Xu wrote:
On Wed, Aug 26, 2020 at 04:56:40PM +0800, shenyang (M) wrote:
@@ -514,13 +514,16 @@ static int hisi_zip_core_debug_init(struct hisi_qm *qm)
struct debugfs_regset32 *regset;
struct dentry *tmp_d;
char buf[HZIP_BUF_SIZE];
- int i;
+ int i, ret;
for (i = 0; i < HZIP_CORE_NUM; i++) {
if (i < HZIP_COMP_CORE_NUM)
- sprintf(buf, "comp_core%d", i);
+ ret = scnprintf(buf, HZIP_BUF_SIZE, "comp_core%d", i);
else
- sprintf(buf, "decomp_core%d", i - HZIP_COMP_CORE_NUM);
+ ret = scnprintf(buf, HZIP_BUF_SIZE, "decomp_core%d",
+ i - HZIP_COMP_CORE_NUM);
+ if (!ret)
+ return -ENOMEM;
and that is just so wrong - did you even try to test
the 'buffer too small' code path?
Do you means the check is unnecessary?
No he's saying that your patch does the wrong thing when the string
is truncated.
Also ENOMEM is a strange error for that case.
Cheers,
Here 'HZIP_BUF_SIZE' is 22, so the buf is enough for the string.
The check for the return is really unnecessary, I will remove it in the
next version.
Thanks,
Yang
Re: [PATCH RESEND 00/10] crypto: hisilicon/zip - misc clean up
any comment? Thanks, Yang On 2020/8/24 11:11, Yang Shen wrote: This patchset make some clean up: patch 1:remove useless parameters patch 4:replace 'sprintf' with 'scnprintf' patch 7:fix static check warning and the rest patch fix some coding style Resend this patch series because it depends on https://patchwork.kernel.org/cover/11715927/ (crypto: hisilicon/qm - misc fixes). Now the patch series(crypto: hisilicon/qm - misc fixes) has been applied. So this patch series will apply against cryptodev. Shukun Tan (1): crypto: hisilicon/zip - modify debugfs interface parameters Yang Shen (9): crypto: hisilicon/zip - remove some useless parameters crypto: hisilicon/zip - unify naming style for functions and macros crypto: hisilicon/zip - replace 'sprintf' with 'scnprintf' crypto: hisilicon/zip - use a enum parameter instead of some macros crypto: hisilicon/zip - add print for error branch crypto: hisilicon/zip - fix static check warning crypto: hisilicon/zip - move some private macros from 'zip.h' to 'zip_crypto.c' crypto: hisilicon/zip - supplement some comments crypto: hisilicon/zip - fix some coding styles drivers/crypto/hisilicon/zip/zip.h| 15 drivers/crypto/hisilicon/zip/zip_crypto.c | 126 - drivers/crypto/hisilicon/zip/zip_main.c | 130 ++ 3 files changed, 148 insertions(+), 123 deletions(-) -- 2.7.4 .
Re: [PATCH RESEND 04/10] crypto: hisilicon/zip - replace 'sprintf' with 'scnprintf'
On 2020/8/24 16:29, David Laight wrote:
From: Yang Shen
Sent: 24 August 2020 04:12
Replace 'sprintf' with 'scnprintf' to avoid overrun.
Signed-off-by: Yang Shen
Reviewed-by: Zhou Wang
---
drivers/crypto/hisilicon/zip/zip_main.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/hisilicon/zip/zip_main.c
b/drivers/crypto/hisilicon/zip/zip_main.c
index df1a16f..1883d1b 100644
--- a/drivers/crypto/hisilicon/zip/zip_main.c
+++ b/drivers/crypto/hisilicon/zip/zip_main.c
@@ -428,7 +428,7 @@ static ssize_t hisi_zip_ctrl_debug_read(struct file *filp,
char __user *buf,
return -EINVAL;
}
spin_unlock_irq(>lock);
- ret = sprintf(tbuf, "%u\n", val);
+ ret = scnprintf(tbuf, HZIP_BUF_SIZE, "%u\n", val);
Should that be sizeof (tbuf).
return simple_read_from_buffer(buf, count, pos, tbuf, ret);
}
@@ -514,13 +514,16 @@ static int hisi_zip_core_debug_init(struct hisi_qm *qm)
struct debugfs_regset32 *regset;
struct dentry *tmp_d;
char buf[HZIP_BUF_SIZE];
- int i;
+ int i, ret;
for (i = 0; i < HZIP_CORE_NUM; i++) {
if (i < HZIP_COMP_CORE_NUM)
- sprintf(buf, "comp_core%d", i);
+ ret = scnprintf(buf, HZIP_BUF_SIZE, "comp_core%d", i);
else
- sprintf(buf, "decomp_core%d", i - HZIP_COMP_CORE_NUM);
+ ret = scnprintf(buf, HZIP_BUF_SIZE, "decomp_core%d",
+ i - HZIP_COMP_CORE_NUM);
+ if (!ret)
+ return -ENOMEM;
and that is just so wrong - did you even try to test
the 'buffer too small' code path?
David
Do you means the check is unnecessary?
Yang
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
.
Re: [v4 01/10] crypto: hisilicon/qm - fix wrong release after using strsep
On 2020/8/6 15:23, Markus Elfring wrote: Would you become interested to look if any other software components would be similarly affected? Yeah, I'll check the rest and fix if any and send those in a clean up patch. Thanks, Yang
Re: [v4 01/10] crypto: hisilicon/qm - fix wrong release after using strsep
On 2020/8/5 14:04, Markus Elfring wrote: Thanks for your review. There is only one error branch need to do something uninit. So I think the jump is not necessary and will affect code reading.:) How does this concern fit to the Linux coding style? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=4da9f3302615f4191814f826054846bf843e24fa#n481 Regards, Markus . Got it, I'll fix this. Thanks, Yang
Re: [PATCH v4 01/10] crypto: hisilicon/qm - fix wrong release after using strsep
On 2020/8/5 2:34, Markus Elfring wrote:
…
+++ b/drivers/crypto/hisilicon/qm.c
@@ -1420,16 +1420,17 @@ static int qm_dbg_help(struct hisi_qm *qm, char *s)
…
+ s_tmp = s;
presult = strsep(, " ");
if (!presult) {
- kfree(s);
+ kfree(s_tmp);
return -EINVAL;
}
- kfree(s);
- return -EINVAL;
+ ret = -EINVAL;
+ goto free_tmp;
I suggest to add a jump target for the desired exception handling.
Regards,
Markus
.
Thanks for your review. There is only one error branch need to do
something uninit. So I think the jump is not necessary and will
affect code reading.:)
Thanks,
Yang
Re: [PATCH 0/4] crypto: hisilicon/zip - misc bugfix
On 2020/7/31 16:28, Herbert Xu wrote: On Sat, Jul 25, 2020 at 02:06:46PM +0800, Yang Shen wrote: This patchset fix some bug: patch 1:clear the debug registers when remove driver patch 2:intercept invalid input when using decompress patch 3:replace the return value '-EBUSY' with '-EAGAIN' when device is busy patch 4:initialize the 'curr_qm_qp_num' when probe device This patchset depends on: https://patchwork.kernel.org/cover/11684785/ Hao Fang (1): crypto: hisilicon/zip - fix the uncleared debug registers Sihang Chen (1): crypto: hisilicon/zip - fix the uninitalized 'curr_qm_qp_num' Yang Shen (1): crypto: hisilicon/zip - fix the return value when device is busy Zhou Wang (1): crypto: hisilicon/zip - fix zero length input in GZIP decompress drivers/crypto/hisilicon/zip/zip_crypto.c | 25 +++-- drivers/crypto/hisilicon/zip/zip_main.c | 19 +++ 2 files changed, 38 insertions(+), 6 deletions(-) This patch series doesn't apply against cryptodev. Cheers, Sorry, this patchset depends on https://patchwork.kernel.org/cover/11684785/ which cover letter is '[PATCH 00/10] crypto: hisilicon/zip - misc clean up' Sorry for troubling you again. Thanks,
Re: [PATCH 03/10] crypto: hisilicon/zip - modify debugfs interface parameters
On 2020/7/31 16:12, Herbert Xu wrote: On Sat, Jul 25, 2020 at 11:44:36AM +0800, Yang Shen wrote: From: Shukun Tan Update debugfs interface parameters Signed-off-by: Shukun Tan Signed-off-by: Yang Shen Reviewed-by: Zhou Wang --- drivers/crypto/hisilicon/zip/zip_main.c | 55 ++--- 1 file changed, 24 insertions(+), 31 deletions(-) This patch doesn't apply against cryptodev. Plesae stop sending patch series that depend on others. Thanks, Sorry, this patchset depends on https://patchwork.kernel.org/cover/11680181/ which cover letter is '[PATCH v3 00/10] crypto: hisilicon/qm - misc fixes' I'm really sorry for the trouble. Thanks,
