| Message ID | 20210316204939.39812-1-m.malygin@yadro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Revert "libfs: fix error cast of negative value in simple_attr_write()" | expand |
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 */ >
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 */