| Message ID | 1366104028-26830-1-git-send-email-bo.li.liu@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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
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
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(-)