On Thu 12-06-25 10:28:55, Wei Gao wrote: > Previously, ext2_fiemap would unconditionally apply "len = min_t(u64, len, > i_size_read(inode));", When inode->i_size was 0 (for an empty file), this > would reduce the requested len to 0. Passing len = 0 to iomap_fiemap could > then result in an -EINVAL error, even for valid queries on empty files. > The new validation logic directly references ext4_fiemap_check_ranges. > > Link: https://github.com/linux-test-project/ltp/issues/1246 > Signed-off-by: Wei Gao <wegao@xxxxxxxx> Thanks for the fix. Some comments below: > diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c > index 30f8201c155f..e5cc61088f21 100644 > --- a/fs/ext2/inode.c > +++ b/fs/ext2/inode.c > @@ -895,10 +895,30 @@ int ext2_fiemap(struct inode *inode, struct fiemap_extent_info *fieinfo, > u64 start, u64 len) > { > int ret; > + u64 maxbytes; > > inode_lock(inode); > - len = min_t(u64, len, i_size_read(inode)); So I agree this 'len' reduction to i_size looks bogus and can be removed. But why are you adding those maxbytes checks when they are done inside iomap_fiemap() shorly later? Honza > + maxbytes = inode->i_sb->s_maxbytes; > + > + if (len == 0) { > + ret = -EINVAL; > + goto unlock_inode; > + } > + > + if (start > maxbytes) { > + ret = -EFBIG; > + goto unlock_inode; > + } > + > + /* > + * Shrink request scope to what the fs can actually handle. > + */ > + if (len > maxbytes || (maxbytes - len) < start) > + len = maxbytes - start; > + > ret = iomap_fiemap(inode, fieinfo, start, len, &ext2_iomap_ops); > + > +unlock_inode: > inode_unlock(inode); > > return ret; > -- > 2.49.0 > -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
