| Message ID | 3990fc5294f2a20a8a4b27c5be0b4b1359f7f1a6.1637618651.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: fail if fstrim_range->start == U64_MAX | expand |
On 2021/11/23 06:04, Josef Bacik wrote: > We've always been failing generic/260 because it's testing things we > actually don't care about and thus won't fail for. However we probably > should fail for fstrim_range->start == U64_MAX since we clearly can't > trim anything past that. This in combination with an update to > generic/260 will allow us to pass this test properly. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/extent-tree.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index 13a371ea68fc..6b4a132d4b86 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -6065,6 +6065,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) > int dev_ret = 0; > int ret = 0; > > + if (range->start == U64_MAX) > + return -EINVAL; > + Isn't the next overflow check would catch anything which length is not U64_MAX? And if length is U64_MAX, we just don't need to trim anything, thus can continue without returing -EINVAL. THanks, Qu > /* > * Check range overflow if range->len is set. > * The default range->len is U64_MAX. >
On Tue, Nov 23, 2021 at 07:29:04AM +0800, Qu Wenruo wrote: > > > On 2021/11/23 06:04, Josef Bacik wrote: > > We've always been failing generic/260 because it's testing things we > > actually don't care about and thus won't fail for. However we probably > > should fail for fstrim_range->start == U64_MAX since we clearly can't > > trim anything past that. This in combination with an update to > > generic/260 will allow us to pass this test properly. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > --- > > fs/btrfs/extent-tree.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > > index 13a371ea68fc..6b4a132d4b86 100644 > > --- a/fs/btrfs/extent-tree.c > > +++ b/fs/btrfs/extent-tree.c > > @@ -6065,6 +6065,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) > > int dev_ret = 0; > > int ret = 0; > > > > + if (range->start == U64_MAX) > > + return -EINVAL; > > + > > Isn't the next overflow check would catch anything which length is not > U64_MAX? > Yes, but if you do range->start == U64_MAX and len == 0 it'll pass. We're being pedantic here, but it's a simple enough sanity check. Thanks, Josef
On Mon, Nov 22, 2021 at 05:04:19PM -0500, Josef Bacik wrote: > We've always been failing generic/260 because it's testing things we > actually don't care about and thus won't fail for. However we probably > should fail for fstrim_range->start == U64_MAX since we clearly can't > trim anything past that. This in combination with an update to > generic/260 will allow us to pass this test properly. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Added to misc-next, thanks.
On 23/11/2021 14:29, Josef Bacik wrote: > On Tue, Nov 23, 2021 at 07:29:04AM +0800, Qu Wenruo wrote: >> >> >> On 2021/11/23 06:04, Josef Bacik wrote: >>> We've always been failing generic/260 because it's testing things we >>> actually don't care about and thus won't fail for. However we probably >>> should fail for fstrim_range->start == U64_MAX since we clearly can't >>> trim anything past that. This in combination with an update to >>> generic/260 will allow us to pass this test properly. >>> >>> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >>> --- >>> fs/btrfs/extent-tree.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c >>> index 13a371ea68fc..6b4a132d4b86 100644 >>> --- a/fs/btrfs/extent-tree.c >>> +++ b/fs/btrfs/extent-tree.c >>> @@ -6065,6 +6065,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) >>> int dev_ret = 0; >>> int ret = 0; >>> >>> + if (range->start == U64_MAX) >>> + return -EINVAL; >>> + >> >> Isn't the next overflow check would catch anything which length is not >> U64_MAX? >> > > Yes, but if you do range->start == U64_MAX and len == 0 it'll pass. We're being > pedantic here, but it's a simple enough sanity check. Thanks, Out of interest, what happens if you do range->start == U64_MAX - 1 and len == 1 ? If that is allowed, why wouldn't range->start == U64_MAX and len == 0 just follow the same code path? Graham
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index 13a371ea68fc..6b4a132d4b86 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -6065,6 +6065,9 @@ int btrfs_trim_fs(struct btrfs_fs_info *fs_info, struct fstrim_range *range) int dev_ret = 0; int ret = 0; + if (range->start == U64_MAX) + return -EINVAL; + /* * Check range overflow if range->len is set. * The default range->len is U64_MAX.
We've always been failing generic/260 because it's testing things we actually don't care about and thus won't fail for. However we probably should fail for fstrim_range->start == U64_MAX since we clearly can't trim anything past that. This in combination with an update to generic/260 will allow us to pass this test properly. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/extent-tree.c | 3 +++ 1 file changed, 3 insertions(+)