| Message ID | 20190814010402.22546-4-jeffm@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] btrfs-progs: mkfs: treat btrfs_add_to_fsid as fatal error | expand |
On 2019/8/14 上午9:04, Jeff Mahoney wrote: > If a user attempts to resize a file system to a size under 256MiB, > it will be rejected with EINVAL and get then unhelpful error message > "ERROR: unable to resize '/path': Invalid argument." > > This commit performs that check before issuing the ioctl to report > a more sensible error message. We also do overflow/underflow > checking when -/+ size is used and report those errors as well. > > Signed-off-by: Jeff Mahoney <jeffm@suse.com> > --- > cmds/filesystem.c | 41 +++++++++++++++++++++++++++++++++++++++++ > common/utils.c | 2 +- > common/utils.h | 2 +- > 3 files changed, 43 insertions(+), 2 deletions(-) > > diff --git a/cmds/filesystem.c b/cmds/filesystem.c > index 4f22089a..e3415126 100644 > --- a/cmds/filesystem.c > +++ b/cmds/filesystem.c > @@ -34,10 +34,12 @@ > #include "kerncompat.h" > #include "ctree.h" > #include "common/utils.h" > +#include "common/device-utils.h" > #include "volumes.h" > #include "cmds/commands.h" > #include "cmds/filesystem-usage.h" > #include "kernel-lib/list_sort.h" > +#include "kernel-lib/overflow.h" > #include "disk-io.h" > #include "common/help.h" > #include "common/fsfeatures.h" > @@ -1062,6 +1064,41 @@ next: > } > static DEFINE_SIMPLE_COMMAND(filesystem_defrag, "defragment"); > > +static int check_resize_size(const char *path, const char *amount) > +{ > + int mod = 0; > + u64 oldsize = 0, size, newsize; > + > + if (*amount == '-') > + mod = -1; > + else if (*amount == '+') > + mod = 1; > + > + if (mod) { > + amount++; > + oldsize = disk_size(path); > + if (oldsize == 0) > + return -1; > + } > + > + size = parse_size(amount); > + > + if (mod == -1 && check_sub_overflow(oldsize, size, &newsize)) { > + error("can't resize to negative size"); > + return -1; > + } else if (mod == 1 && check_add_overflow(oldsize, size, &newsize)) { > + error("can't resize to larger than 16EiB"); > + return -1; > + } else > + newsize = size; > + > + if (newsize < SZ_256M) { > + error("can't resize to size smaller than 256MiB"); > + return -1; > + } > + return 0; > +} > + > static const char * const cmd_filesystem_resize_usage[] = { > "btrfs filesystem resize [devid:][+/-]<newsize>[kKmMgGtTpPeE]|[devid:]max <path>", > "Resize a filesystem", > @@ -1110,6 +1147,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd, > if (fd < 0) > return 1; > > + res = check_resize_size(path, amount); > + if (res < 0) > + return 1; > + > printf("Resize '%s' of '%s'\n", path, amount); > memset(&args, 0, sizeof(args)); > strncpy_null(args.name, amount); > diff --git a/common/utils.c b/common/utils.c > index ad938409..f2a10ccc 100644 > --- a/common/utils.c > +++ b/common/utils.c > @@ -638,7 +638,7 @@ static int fls64(u64 x) > return 64 - i; > } > > -u64 parse_size(char *s) > +u64 parse_size(const char *s) Although a good change, not sure if David will ask for an explict patch for that. Despite that, looks good. Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > { > char c; > char *endptr; > diff --git a/common/utils.h b/common/utils.h > index 7867fb0a..0ef1d6e8 100644 > --- a/common/utils.h > +++ b/common/utils.h > @@ -65,7 +65,7 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, unsigned unit_mo > #define pretty_size(size) pretty_size_mode(size, UNITS_DEFAULT) > const char *pretty_size_mode(u64 size, unsigned mode); > > -u64 parse_size(char *s); > +u64 parse_size(const char *s); > u64 parse_qgroupid(const char *p); > u64 arg_strtou64(const char *str); > int open_file_or_dir(const char *fname, DIR **dirstream); >
On Wed, Aug 14, 2019 at 09:53:33AM +0800, Qu Wenruo wrote: > > > On 2019/8/14 上午9:04, Jeff Mahoney wrote: > > If a user attempts to resize a file system to a size under 256MiB, > > it will be rejected with EINVAL and get then unhelpful error message > > "ERROR: unable to resize '/path': Invalid argument." > > > > This commit performs that check before issuing the ioctl to report > > a more sensible error message. We also do overflow/underflow > > checking when -/+ size is used and report those errors as well. > > > > Signed-off-by: Jeff Mahoney <jeffm@suse.com> > > --- > > cmds/filesystem.c | 41 +++++++++++++++++++++++++++++++++++++++++ > > common/utils.c | 2 +- > > common/utils.h | 2 +- > > 3 files changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/cmds/filesystem.c b/cmds/filesystem.c > > index 4f22089a..e3415126 100644 > > --- a/cmds/filesystem.c > > +++ b/cmds/filesystem.c > > @@ -34,10 +34,12 @@ > > #include "kerncompat.h" > > #include "ctree.h" > > #include "common/utils.h" > > +#include "common/device-utils.h" > > #include "volumes.h" > > #include "cmds/commands.h" > > #include "cmds/filesystem-usage.h" > > #include "kernel-lib/list_sort.h" > > +#include "kernel-lib/overflow.h" > > #include "disk-io.h" > > #include "common/help.h" > > #include "common/fsfeatures.h" > > @@ -1062,6 +1064,41 @@ next: > > } > > static DEFINE_SIMPLE_COMMAND(filesystem_defrag, "defragment"); > > > > +static int check_resize_size(const char *path, const char *amount) > > +{ > > + int mod = 0; > > + u64 oldsize = 0, size, newsize; > > + > > + if (*amount == '-') > > + mod = -1; > > + else if (*amount == '+') > > + mod = 1; > > + > > + if (mod) { > > + amount++; > > + oldsize = disk_size(path); > > + if (oldsize == 0) > > + return -1; > > + } > > + > > + size = parse_size(amount); > > + > > + if (mod == -1 && check_sub_overflow(oldsize, size, &newsize)) { > > + error("can't resize to negative size"); > > + return -1; > > + } else if (mod == 1 && check_add_overflow(oldsize, size, &newsize)) { > > + error("can't resize to larger than 16EiB"); > > + return -1; > > + } else > > + newsize = size; > > + > > + if (newsize < SZ_256M) { > > + error("can't resize to size smaller than 256MiB"); > > + return -1; > > + } > > + return 0; > > +} > > + > > static const char * const cmd_filesystem_resize_usage[] = { > > "btrfs filesystem resize [devid:][+/-]<newsize>[kKmMgGtTpPeE]|[devid:]max <path>", > > "Resize a filesystem", > > @@ -1110,6 +1147,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd, > > if (fd < 0) > > return 1; > > > > + res = check_resize_size(path, amount); > > + if (res < 0) > > + return 1; > > + > > printf("Resize '%s' of '%s'\n", path, amount); > > memset(&args, 0, sizeof(args)); > > strncpy_null(args.name, amount); > > diff --git a/common/utils.c b/common/utils.c > > index ad938409..f2a10ccc 100644 > > --- a/common/utils.c > > +++ b/common/utils.c > > @@ -638,7 +638,7 @@ static int fls64(u64 x) > > return 64 - i; > > } > > > > -u64 parse_size(char *s) > > +u64 parse_size(const char *s) > > Although a good change, not sure if David will ask for an explict patch > for that. I've split that from the patch. > Despite that, looks good. Well, no. The resize specifier is more complex and has eg. formats like 1:+1G, max, 2:-2G, 3:max. Moreover the relative change must not be compared to the 256M limit, 'resize -128M' fails while it should not for a filesystem that's eg. 2G.
diff --git a/cmds/filesystem.c b/cmds/filesystem.c index 4f22089a..e3415126 100644 --- a/cmds/filesystem.c +++ b/cmds/filesystem.c @@ -34,10 +34,12 @@ #include "kerncompat.h" #include "ctree.h" #include "common/utils.h" +#include "common/device-utils.h" #include "volumes.h" #include "cmds/commands.h" #include "cmds/filesystem-usage.h" #include "kernel-lib/list_sort.h" +#include "kernel-lib/overflow.h" #include "disk-io.h" #include "common/help.h" #include "common/fsfeatures.h" @@ -1062,6 +1064,41 @@ next: } static DEFINE_SIMPLE_COMMAND(filesystem_defrag, "defragment"); +static int check_resize_size(const char *path, const char *amount) +{ + int mod = 0; + u64 oldsize = 0, size, newsize; + + if (*amount == '-') + mod = -1; + else if (*amount == '+') + mod = 1; + + if (mod) { + amount++; + oldsize = disk_size(path); + if (oldsize == 0) + return -1; + } + + size = parse_size(amount); + + if (mod == -1 && check_sub_overflow(oldsize, size, &newsize)) { + error("can't resize to negative size"); + return -1; + } else if (mod == 1 && check_add_overflow(oldsize, size, &newsize)) { + error("can't resize to larger than 16EiB"); + return -1; + } else + newsize = size; + + if (newsize < SZ_256M) { + error("can't resize to size smaller than 256MiB"); + return -1; + } + return 0; +} + static const char * const cmd_filesystem_resize_usage[] = { "btrfs filesystem resize [devid:][+/-]<newsize>[kKmMgGtTpPeE]|[devid:]max <path>", "Resize a filesystem", @@ -1110,6 +1147,10 @@ static int cmd_filesystem_resize(const struct cmd_struct *cmd, if (fd < 0) return 1; + res = check_resize_size(path, amount); + if (res < 0) + return 1; + printf("Resize '%s' of '%s'\n", path, amount); memset(&args, 0, sizeof(args)); strncpy_null(args.name, amount); diff --git a/common/utils.c b/common/utils.c index ad938409..f2a10ccc 100644 --- a/common/utils.c +++ b/common/utils.c @@ -638,7 +638,7 @@ static int fls64(u64 x) return 64 - i; } -u64 parse_size(char *s) +u64 parse_size(const char *s) { char c; char *endptr; diff --git a/common/utils.h b/common/utils.h index 7867fb0a..0ef1d6e8 100644 --- a/common/utils.h +++ b/common/utils.h @@ -65,7 +65,7 @@ int pretty_size_snprintf(u64 size, char *str, size_t str_bytes, unsigned unit_mo #define pretty_size(size) pretty_size_mode(size, UNITS_DEFAULT) const char *pretty_size_mode(u64 size, unsigned mode); -u64 parse_size(char *s); +u64 parse_size(const char *s); u64 parse_qgroupid(const char *p); u64 arg_strtou64(const char *str); int open_file_or_dir(const char *fname, DIR **dirstream);
If a user attempts to resize a file system to a size under 256MiB, it will be rejected with EINVAL and get then unhelpful error message "ERROR: unable to resize '/path': Invalid argument." This commit performs that check before issuing the ioctl to report a more sensible error message. We also do overflow/underflow checking when -/+ size is used and report those errors as well. Signed-off-by: Jeff Mahoney <jeffm@suse.com> --- cmds/filesystem.c | 41 +++++++++++++++++++++++++++++++++++++++++ common/utils.c | 2 +- common/utils.h | 2 +- 3 files changed, 43 insertions(+), 2 deletions(-)