| Message ID | 2693b00ca850b0f604e03c836e71d0ad8a93ffee.1702628925.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lib/kstrtox: introduce kstrtoull_suffix() helper | expand |
Hi Qu, On Fri, 15 Dec 2023 19:09:24 +1030, Qu Wenruo wrote: > Since memparse() itself can not handle overflow at all, use > memparse_ull() to be extra safe. s/memparse_ull/kstrtoull_suffix/ > Now overflow values can be properly detected. Please document how the sysfs API changes with this, in addition to overflow handling: - support for 'E' / 'e' suffixes dropped - only one trailing '\n' accepted, instead of many isspace() The latter might break a few scripts. Cheers, David > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > fs/btrfs/sysfs.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c > index 84c05246ffd8..089c3fc123fe 100644 > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -760,7 +760,7 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, > { > struct btrfs_space_info *space_info = to_space_info(kobj); > struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj)); > - char *retptr; > + int ret; > u64 val; > > if (!capable(CAP_SYS_ADMIN)) > @@ -776,11 +776,9 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, > if (space_info->flags & BTRFS_BLOCK_GROUP_SYSTEM) > return -EPERM; > > - val = memparse(buf, &retptr); > - /* There could be trailing '\n', also catch any typos after the value */ > - retptr = skip_spaces(retptr); > - if (*retptr != 0 || val == 0) > - return -EINVAL; > + ret = kstrtoull_suffix(buf, 0, &val, KSTRTOULL_SUFFIX_DEFAULT); > + if (ret < 0) > + return ret; > > val = min(val, BTRFS_MAX_DATA_CHUNK_SIZE); > > @@ -1779,14 +1777,12 @@ static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj, > { > struct btrfs_device *device = container_of(kobj, struct btrfs_device, > devid_kobj); > - char *endptr; > unsigned long long limit; > + int ret; > > - limit = memparse(buf, &endptr); > - /* There could be trailing '\n', also catch any typos after the value. */ > - endptr = skip_spaces(endptr); > - if (*endptr != 0) > - return -EINVAL; > + ret = kstrtoull_suffix(buf, 0, &limit, KSTRTOULL_SUFFIX_DEFAULT); > + if (ret < 0) > + return ret; > WRITE_ONCE(device->scrub_speed_max, limit); > return len; > }
On 2023/12/18 18:19, David Disseldorp wrote: > Hi Qu, > > On Fri, 15 Dec 2023 19:09:24 +1030, Qu Wenruo wrote: > >> Since memparse() itself can not handle overflow at all, use >> memparse_ull() to be extra safe. > > s/memparse_ull/kstrtoull_suffix/ > >> Now overflow values can be properly detected. > > Please document how the sysfs API changes with this, in addition to > overflow handling: > - support for 'E' / 'e' suffixes dropped > - only one trailing '\n' accepted, instead of many isspace() Well, multiple spaces are already an abuse, and I don't believe sane scripts should go multiple spaces/newlines. As all the other call sites are going kstrtox, which only accept one newline. Although the change is indeed worthy a document update. Thanks, Qu > > The latter might break a few scripts. > > Cheers, David > >> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> fs/btrfs/sysfs.c | 20 ++++++++------------ >> 1 file changed, 8 insertions(+), 12 deletions(-) >> >> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c >> index 84c05246ffd8..089c3fc123fe 100644 >> --- a/fs/btrfs/sysfs.c >> +++ b/fs/btrfs/sysfs.c >> @@ -760,7 +760,7 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, >> { >> struct btrfs_space_info *space_info = to_space_info(kobj); >> struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj)); >> - char *retptr; >> + int ret; >> u64 val; >> >> if (!capable(CAP_SYS_ADMIN)) >> @@ -776,11 +776,9 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, >> if (space_info->flags & BTRFS_BLOCK_GROUP_SYSTEM) >> return -EPERM; >> >> - val = memparse(buf, &retptr); >> - /* There could be trailing '\n', also catch any typos after the value */ >> - retptr = skip_spaces(retptr); >> - if (*retptr != 0 || val == 0) >> - return -EINVAL; >> + ret = kstrtoull_suffix(buf, 0, &val, KSTRTOULL_SUFFIX_DEFAULT); >> + if (ret < 0) >> + return ret; >> >> val = min(val, BTRFS_MAX_DATA_CHUNK_SIZE); >> >> @@ -1779,14 +1777,12 @@ static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj, >> { >> struct btrfs_device *device = container_of(kobj, struct btrfs_device, >> devid_kobj); >> - char *endptr; >> unsigned long long limit; >> + int ret; >> >> - limit = memparse(buf, &endptr); >> - /* There could be trailing '\n', also catch any typos after the value. */ >> - endptr = skip_spaces(endptr); >> - if (*endptr != 0) >> - return -EINVAL; >> + ret = kstrtoull_suffix(buf, 0, &limit, KSTRTOULL_SUFFIX_DEFAULT); >> + if (ret < 0) >> + return ret; >> WRITE_ONCE(device->scrub_speed_max, limit); >> return len; >> } > > > >
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 84c05246ffd8..089c3fc123fe 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -760,7 +760,7 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, { struct btrfs_space_info *space_info = to_space_info(kobj); struct btrfs_fs_info *fs_info = to_fs_info(get_btrfs_kobj(kobj)); - char *retptr; + int ret; u64 val; if (!capable(CAP_SYS_ADMIN)) @@ -776,11 +776,9 @@ static ssize_t btrfs_chunk_size_store(struct kobject *kobj, if (space_info->flags & BTRFS_BLOCK_GROUP_SYSTEM) return -EPERM; - val = memparse(buf, &retptr); - /* There could be trailing '\n', also catch any typos after the value */ - retptr = skip_spaces(retptr); - if (*retptr != 0 || val == 0) - return -EINVAL; + ret = kstrtoull_suffix(buf, 0, &val, KSTRTOULL_SUFFIX_DEFAULT); + if (ret < 0) + return ret; val = min(val, BTRFS_MAX_DATA_CHUNK_SIZE); @@ -1779,14 +1777,12 @@ static ssize_t btrfs_devinfo_scrub_speed_max_store(struct kobject *kobj, { struct btrfs_device *device = container_of(kobj, struct btrfs_device, devid_kobj); - char *endptr; unsigned long long limit; + int ret; - limit = memparse(buf, &endptr); - /* There could be trailing '\n', also catch any typos after the value. */ - endptr = skip_spaces(endptr); - if (*endptr != 0) - return -EINVAL; + ret = kstrtoull_suffix(buf, 0, &limit, KSTRTOULL_SUFFIX_DEFAULT); + if (ret < 0) + return ret; WRITE_ONCE(device->scrub_speed_max, limit); return len; }
Since memparse() itself can not handle overflow at all, use memparse_ull() to be extra safe. Now overflow values can be properly detected. Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/sysfs.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-)