* [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs
@ 2020-05-28 12:37 Dan Carpenter
2020-05-29 6:24 ` Zhou Wang
2020-06-01 2:19 ` Shukun Tan
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-05-28 12:37 UTC (permalink / raw)
To: Zhou Wang
Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel, kernel-janitors
Originally this code rejected any read less than 256 bytes. There
is no need for this artificial limit.
Also I have changed the snprintf() functions to scnprintf(). The
difference is that snprintf() returns the number of bytes which would
have been copied if there were enough space and scnprintf() returns the
number of bytes which were actually copied. It doesn't matter here
because the strings are very short so they can't go over 256 bytes.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/crypto/hisilicon/qm.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
index a781c02251980..9c0c9f500d91d 100644
--- a/drivers/crypto/hisilicon/qm.c
+++ b/drivers/crypto/hisilicon/qm.c
@@ -1076,16 +1076,15 @@ static ssize_t qm_cmd_read(struct file *filp, char __user *buffer,
if (*pos)
return 0;
- if (count < QM_DBG_READ_LEN)
- return -ENOSPC;
-
- len = snprintf(buf, QM_DBG_READ_LEN, "%s\n",
+ len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n",
"Please echo help to cmd to get help information");
+ len = min_t(size_t, len, count);
if (copy_to_user(buffer, buf, len))
return -EFAULT;
- return (*pos = len);
+ *pos = len;
+ return len;
}
static void *qm_ctx_alloc(struct hisi_qm *qm, size_t ctx_size,
@@ -2710,19 +2709,18 @@ static ssize_t qm_status_read(struct file *filp, char __user *buffer,
if (*pos)
return 0;
- if (count < QM_DBG_READ_LEN)
- return -ENOSPC;
-
val = atomic_read(&qm->status.flags);
- len = snprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
+ len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
if (!len)
return -EFAULT;
+ len = min_t(size_t, len, count);
cp_len = copy_to_user(buffer, buf, len);
if (cp_len)
return -EFAULT;
- return (*pos = len);
+ *pos = len;
+ return len;
}
static const struct file_operations qm_status_fops = {
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs
2020-05-28 12:37 [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs Dan Carpenter
@ 2020-05-29 6:24 ` Zhou Wang
2020-06-01 2:19 ` Shukun Tan
1 sibling, 0 replies; 3+ messages in thread
From: Zhou Wang @ 2020-05-29 6:24 UTC (permalink / raw)
To: Dan Carpenter
Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel, kernel-janitors
On 2020/5/28 20:37, Dan Carpenter wrote:
> Originally this code rejected any read less than 256 bytes. There
> is no need for this artificial limit.
>
> Also I have changed the snprintf() functions to scnprintf(). The
> difference is that snprintf() returns the number of bytes which would
> have been copied if there were enough space and scnprintf() returns the
> number of bytes which were actually copied. It doesn't matter here
> because the strings are very short so they can't go over 256 bytes.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Looks good to me, thanks!
Reviewed-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
> drivers/crypto/hisilicon/qm.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
> index a781c02251980..9c0c9f500d91d 100644
> --- a/drivers/crypto/hisilicon/qm.c
> +++ b/drivers/crypto/hisilicon/qm.c
> @@ -1076,16 +1076,15 @@ static ssize_t qm_cmd_read(struct file *filp, char __user *buffer,
> if (*pos)
> return 0;
>
> - if (count < QM_DBG_READ_LEN)
> - return -ENOSPC;
> -
> - len = snprintf(buf, QM_DBG_READ_LEN, "%s\n",
> + len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n",
> "Please echo help to cmd to get help information");
>
> + len = min_t(size_t, len, count);
> if (copy_to_user(buffer, buf, len))
> return -EFAULT;
>
> - return (*pos = len);
> + *pos = len;
> + return len;
> }
>
> static void *qm_ctx_alloc(struct hisi_qm *qm, size_t ctx_size,
> @@ -2710,19 +2709,18 @@ static ssize_t qm_status_read(struct file *filp, char __user *buffer,
> if (*pos)
> return 0;
>
> - if (count < QM_DBG_READ_LEN)
> - return -ENOSPC;
> -
> val = atomic_read(&qm->status.flags);
> - len = snprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
> + len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
> if (!len)
> return -EFAULT;
>
> + len = min_t(size_t, len, count);
> cp_len = copy_to_user(buffer, buf, len);
> if (cp_len)
> return -EFAULT;
>
> - return (*pos = len);
> + *pos = len;
> + return len;
> }
>
> static const struct file_operations qm_status_fops = {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs
2020-05-28 12:37 [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs Dan Carpenter
2020-05-29 6:24 ` Zhou Wang
@ 2020-06-01 2:19 ` Shukun Tan
1 sibling, 0 replies; 3+ messages in thread
From: Shukun Tan @ 2020-06-01 2:19 UTC (permalink / raw)
To: Dan Carpenter, Zhou Wang
Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel, kernel-janitors
Hi Dan & Zhou,
On 2020/5/28 20:37, Dan Carpenter wrote:
> Originally this code rejected any read less than 256 bytes. There
> is no need for this artificial limit.
>
> Also I have changed the snprintf() functions to scnprintf(). The
> difference is that snprintf() returns the number of bytes which would
> have been copied if there were enough space and scnprintf() returns the
> number of bytes which were actually copied. It doesn't matter here
> because the strings are very short so they can't go over 256 bytes.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/crypto/hisilicon/qm.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/crypto/hisilicon/qm.c b/drivers/crypto/hisilicon/qm.c
> index a781c02251980..9c0c9f500d91d 100644
> --- a/drivers/crypto/hisilicon/qm.c
> +++ b/drivers/crypto/hisilicon/qm.c
> @@ -1076,16 +1076,15 @@ static ssize_t qm_cmd_read(struct file *filp, char __user *buffer,
> if (*pos)
> return 0;
>
> - if (count < QM_DBG_READ_LEN)
> - return -ENOSPC;
> -
I think we should keep it, maybe you are right when using 'cat', but if you use 'read'
system call this may make the user read only part of the prompt. This may break our
original design. :)
Thanks,
Shukun
> - len = snprintf(buf, QM_DBG_READ_LEN, "%s\n",
> + len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n",
> "Please echo help to cmd to get help information");
>
> + len = min_t(size_t, len, count);
> if (copy_to_user(buffer, buf, len))
> return -EFAULT;
>
> - return (*pos = len);
> + *pos = len;
> + return len;
> }
>
> static void *qm_ctx_alloc(struct hisi_qm *qm, size_t ctx_size,
> @@ -2710,19 +2709,18 @@ static ssize_t qm_status_read(struct file *filp, char __user *buffer,
> if (*pos)
> return 0;
>
> - if (count < QM_DBG_READ_LEN)
> - return -ENOSPC;
> -
> val = atomic_read(&qm->status.flags);
> - len = snprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
> + len = scnprintf(buf, QM_DBG_READ_LEN, "%s\n", qm_s[val]);
> if (!len)
> return -EFAULT;
>
> + len = min_t(size_t, len, count);
> cp_len = copy_to_user(buffer, buf, len);
> if (cp_len)
> return -EFAULT;
>
> - return (*pos = len);
> + *pos = len;
> + return len;
> }
>
> static const struct file_operations qm_status_fops = {
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-01 2:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 12:37 [PATCH] crypto: hisilicon/qm - allow smaller reads in debugfs Dan Carpenter
2020-05-29 6:24 ` Zhou Wang
2020-06-01 2:19 ` Shukun Tan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).