Liu Bo
2013-Apr-16 09:20 UTC
[PATCH v2] Btrfs: return error when we specify wrong start to defrag
We need such a sanity check for wrong start when we defrag a file, otherwise, even with a wrong start that''s larger than file size, we can end up changing not only inode''s force compress flag but also FS''s incompat flags. Signed-off-by: Liu Bo <bo.li.liu@oracle.com> --- v2: make changelog more clearly. fs/btrfs/ioctl.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 69cd80d..262d9db 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -1152,8 +1152,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, u64 new_align = ~((u64)128 * 1024 - 1); struct page **pages = NULL; - if (extent_thresh == 0) - extent_thresh = 256 * 1024; + if (isize == 0) + return 0; + + if (range->start >= isize) + return -EINVAL; if (range->flags & BTRFS_DEFRAG_RANGE_COMPRESS) { if (range->compress_type > BTRFS_COMPRESS_TYPES) @@ -1162,8 +1165,8 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, compress_type = range->compress_type; } - if (isize == 0) - return 0; + if (extent_thresh == 0) + extent_thresh = 256 * 1024; /* * if we were not given a file, allocate a readahead -- 1.7.7 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
David Sterba
2013-Apr-16 13:38 UTC
Re: [PATCH v2] Btrfs: return error when we specify wrong start to defrag
On Tue, Apr 16, 2013 at 05:20:28PM +0800, Liu Bo wrote:> We need such a sanity check for wrong start when we defrag a file, otherwise, > even with a wrong start that''s larger than file size, we can end up changing > not only inode''s force compress flag but also FS''s incompat flags.The range->start check is good, but why are you worried about the incompat flag? LZO support has been around for more than 2 years.> --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -1152,8 +1152,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, > u64 new_align = ~((u64)128 * 1024 - 1); > struct page **pages = NULL; > > - if (extent_thresh == 0) > - extent_thresh = 256 * 1024; > + if (isize == 0) > + return 0; > + > + if (range->start >= isize) > + return -EINVAL;...> - if (isize == 0) > - return 0; > + if (extent_thresh == 0) > + extent_thresh = 256 * 1024;That''s a more logical order of the checks, good. Reviewed-by: David Sterba <dsterba@suse.cz> -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Liu Bo
2013-Apr-16 13:49 UTC
Re: [PATCH v2] Btrfs: return error when we specify wrong start to defrag
On Tue, Apr 16, 2013 at 03:38:28PM +0200, David Sterba wrote:> On Tue, Apr 16, 2013 at 05:20:28PM +0800, Liu Bo wrote: > > We need such a sanity check for wrong start when we defrag a file, otherwise, > > even with a wrong start that''s larger than file size, we can end up changing > > not only inode''s force compress flag but also FS''s incompat flags. > > The range->start check is good, but why are you worried about the > incompat flag? LZO support has been around for more than 2 years.As the code of setting LZO incompat flags is just there, so I take it as a side effect. Well, I''m not worried now :)> > > --- a/fs/btrfs/ioctl.c > > +++ b/fs/btrfs/ioctl.c > > @@ -1152,8 +1152,11 @@ int btrfs_defrag_file(struct inode *inode, struct file *file, > > u64 new_align = ~((u64)128 * 1024 - 1); > > struct page **pages = NULL; > > > > - if (extent_thresh == 0) > > - extent_thresh = 256 * 1024; > > + if (isize == 0) > > + return 0; > > + > > + if (range->start >= isize) > > + return -EINVAL; > ... > > - if (isize == 0) > > - return 0; > > + if (extent_thresh == 0) > > + extent_thresh = 256 * 1024; > > That''s a more logical order of the checks, good. > > Reviewed-by: David Sterba <dsterba@suse.cz>Thanks for the quick review! thanks, liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Maybe Matching Threads
- [Btrfs-Progs] Update for lzo support
- [PATCH v4] Btrfs: Check INCOMPAT flags on remount and add helper function
- [PATCH] Ignore unfragmented file checks in defrag when compression enabled
- [PATCH 0/5] btrfs: lz4/lz4hc compression
- [RFB] add LZ4 compression method to btrfs