* [PATCH] Revert "libfs: fix error cast of negative value in simple_attr_write()"
@ 2021-03-16 20:49 m.malygin
2021-03-17 3:38 ` Yicong Yang
0 siblings, 1 reply; 2+ messages in thread
From: m.malygin @ 2021-03-16 20:49 UTC (permalink / raw)
To: linux-fsdevel; +Cc: r.bolshakov, yangyicong, linux, Mikhail Malygin
From: Mikhail Malygin <m.malygin@yadro.com>
This reverts commit 488dac0c9237647e9b8f788b6a342595bfa40bda.
An established and documented [1] way of of configuring unlimited number of failures in fault-injection framework is to write -1:
- /sys/kernel/debug/fail*/times:
specifies how many times failures may happen at most.
A value of -1 means "no limit".
Commit 488dac0c92 inadverently breaks that.
1. https://www.kernel.org/doc/Documentation/fault-injection/fault-injection.txt
Signed-off-by: Mikhail Malygin <m.malygin@yadro.com>
Signef-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
fs/libfs.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/libfs.c b/fs/libfs.c
index e2de5401abca..9bea71111299 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -961,7 +961,7 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
size_t len, loff_t *ppos)
{
struct simple_attr *attr;
- unsigned long long val;
+ u64 val;
size_t size;
ssize_t ret;
@@ -979,9 +979,7 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
goto out;
attr->set_buf[size] = '\0';
- ret = kstrtoull(attr->set_buf, 0, &val);
- if (ret)
- goto out;
+ val = simple_strtoll(attr->set_buf, NULL, 0);
ret = attr->set(attr->data, val);
if (ret == 0)
ret = len; /* on success, claim we got the whole input */
--
2.24.3
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] Revert "libfs: fix error cast of negative value in simple_attr_write()"
2021-03-16 20:49 [PATCH] Revert "libfs: fix error cast of negative value in simple_attr_write()" m.malygin
@ 2021-03-17 3:38 ` Yicong Yang
0 siblings, 0 replies; 2+ messages in thread
From: Yicong Yang @ 2021-03-17 3:38 UTC (permalink / raw)
To: m.malygin, linux-fsdevel; +Cc: r.bolshakov, linux
Hi Mikhail,
On 2021/3/17 4:49, m.malygin@yadro.com wrote:
> From: Mikhail Malygin <m.malygin@yadro.com>
>
> This reverts commit 488dac0c9237647e9b8f788b6a342595bfa40bda.
>
> An established and documented [1] way of of configuring unlimited number of failures in fault-injection framework is to write -1:
>
> - /sys/kernel/debug/fail*/times:
>
> specifies how many times failures may happen at most.
> A value of -1 means "no limit".
>
> Commit 488dac0c92 inadverently breaks that.
>
> 1. https://www.kernel.org/doc/Documentation/fault-injection/fault-injection.txt
a simple revert can address this issue, but i don't think it's reasonable to directly cast the negative value.
considering attr->set() callback receives a value of u64, it's hard for the upper users
to know whether it's casted or not.
>
> Signed-off-by: Mikhail Malygin <m.malygin@yadro.com>
> Signef-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> fs/libfs.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/fs/libfs.c b/fs/libfs.c
> index e2de5401abca..9bea71111299 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -961,7 +961,7 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
> size_t len, loff_t *ppos)
> {
> struct simple_attr *attr;
> - unsigned long long val;
> + u64 val;
> size_t size;
> ssize_t ret;
>
> @@ -979,9 +979,7 @@ ssize_t simple_attr_write(struct file *file, const char __user *buf,
> goto out;
>
> attr->set_buf[size] = '\0';
> - ret = kstrtoull(attr->set_buf, 0, &val);
> - if (ret)
> - goto out;
> + val = simple_strtoll(attr->set_buf, NULL, 0);
simple_strtoll() is deprecated and has unexpected results[1],
use kstrtoll() instead.
[1] https://github.com/torvalds/linux/blob/master/Documentation/process/deprecated.rst#simple_strtol-simple_strtoll-simple_strtoul-simple_strtoull
Thanks,
Yicong
> ret = attr->set(attr->data, val);
> if (ret == 0)
> ret = len; /* on success, claim we got the whole input */
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-03-17 3:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 20:49 [PATCH] Revert "libfs: fix error cast of negative value in simple_attr_write()" m.malygin
2021-03-17 3:38 ` Yicong Yang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.