| Message ID | 20220315065745.3441989-1-guoxuenan@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iomap: fix an infinite loop in iomap_fiemap | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
Hi Xuenan, (+ Cc Darrick.. ) On Tue, Mar 15, 2022 at 02:57:45PM +0800, Guo Xuenan wrote: > when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start > will always true , then *len set zero. because of start offset is byhond > file size, for erofs filesystem it will always return iomap.length with > zero,iomap iterate will be infinite loop. Thanks! If my understanding is correct, we once had a similar behavior in compressed inodes and it has been worked around with https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/erofs/zmap.c?h=v5.17-rc8#n762 But yeah, if iomap can define or handle post-EOF extent length behavior exactly, it would be much better! So on my side, Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> (Also you might need to inform iomap maintainer Darrick as well..) Thanks, Gao Xiang > > In order to avoid this situation, it is better to calculate the actual > mapping length at first. If the len is 0, there is no need to continue > the operation. > > ------------[ cut here ]------------ > WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 > Modules linked in: xfs erofs > CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > RIP: 0010:iomap_iter+0x97f/0xc70 > Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be > RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 > RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 > RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 > RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 > R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff > R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 > FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > iomap_fiemap+0x1c9/0x2f0 > erofs_fiemap+0x64/0x90 [erofs] > do_vfs_ioctl+0x40d/0x12e0 > __x64_sys_ioctl+0xaa/0x1c0 > do_syscall_64+0x35/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > </TASK> > ---[ end trace 0000000000000000 ]--- > watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> > --- > fs/ioctl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/ioctl.c b/fs/ioctl.c > index 1ed097e94af2..7f70e90766ed 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > u32 incompat_flags; > int ret = 0; > > - if (*len == 0) > - return -EINVAL; > if (start > maxbytes) > return -EFBIG; > > @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > if (*len > maxbytes || (maxbytes - *len) < start) > *len = maxbytes - start; > > + if (*len == 0) > + return -EINVAL; > + > supported_flags |= FIEMAP_FLAG_SYNC; > supported_flags &= FIEMAP_FLAGS_COMPAT; > incompat_flags = fieinfo->fi_flags & ~supported_flags; > -- > 2.22.0
Hi, 在 2022/3/15 16:04, Gao Xiang 写道: > Hi Xuenan, > > (+ Cc Darrick.. ) > > On Tue, Mar 15, 2022 at 02:57:45PM +0800, Guo Xuenan wrote: >> when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start >> will always true , then *len set zero. because of start offset is byhond >> file size, for erofs filesystem it will always return iomap.length with >> zero,iomap iterate will be infinite loop. > Thanks! If my understanding is correct, we once had a similar > behavior in compressed inodes and it has been worked around with > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/erofs/zmap.c?h=v5.17-rc8#n762 Yes, your understanding is accurate. But non-compressed inode,still has the same problem, In my testcase, set filemap start offset "0x7fffffffffffffff", filemap len will always be reset to 0; https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/erofs/data.c?h=v5.17-rc8#n267 will always set iomap->length. But i don't think this is the problem of erofs. In my opinion, for this situation, when filemap len is 0, there is no point to enter a specific filesystem. Thanks! > But yeah, if iomap can define or handle post-EOF extent length > behavior exactly, it would be much better! > > So on my side, > > Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com> > > (Also you might need to inform iomap maintainer Darrick as well..) > > Thanks, > Gao Xiang > > >> In order to avoid this situation, it is better to calculate the actual >> mapping length at first. If the len is 0, there is no need to continue >> the operation. >> >> ------------[ cut here ]------------ >> WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 >> Modules linked in: xfs erofs >> CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >> RIP: 0010:iomap_iter+0x97f/0xc70 >> Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be >> RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 >> RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 >> RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 >> RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 >> R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff >> R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 >> FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Call Trace: >> <TASK> >> iomap_fiemap+0x1c9/0x2f0 >> erofs_fiemap+0x64/0x90 [erofs] >> do_vfs_ioctl+0x40d/0x12e0 >> __x64_sys_ioctl+0xaa/0x1c0 >> do_syscall_64+0x35/0x80 >> entry_SYSCALL_64_after_hwframe+0x44/0xae >> </TASK> >> ---[ end trace 0000000000000000 ]--- >> watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> >> --- >> fs/ioctl.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ioctl.c b/fs/ioctl.c >> index 1ed097e94af2..7f70e90766ed 100644 >> --- a/fs/ioctl.c >> +++ b/fs/ioctl.c >> @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, >> u32 incompat_flags; >> int ret = 0; >> >> - if (*len == 0) >> - return -EINVAL; >> if (start > maxbytes) >> return -EFBIG; >> >> @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, >> if (*len > maxbytes || (maxbytes - *len) < start) >> *len = maxbytes - start; >> >> + if (*len == 0) >> + return -EINVAL; >> + >> supported_flags |= FIEMAP_FLAG_SYNC; >> supported_flags &= FIEMAP_FLAGS_COMPAT; >> incompat_flags = fieinfo->fi_flags & ~supported_flags; >> -- >> 2.22.0 > .
On Tue, Mar 15, 2022 at 02:57:45PM +0800, Guo Xuenan wrote: > when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start > will always true , then *len set zero. because of start offset is byhond > file size, for erofs filesystem it will always return iomap.length with > zero,iomap iterate will be infinite loop. > > In order to avoid this situation, it is better to calculate the actual > mapping length at first. If the len is 0, there is no need to continue > the operation. > > ------------[ cut here ]------------ > WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 > Modules linked in: xfs erofs > CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > RIP: 0010:iomap_iter+0x97f/0xc70 > Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be > RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 > RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 > RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 > RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 > R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff > R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 > FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Call Trace: > <TASK> > iomap_fiemap+0x1c9/0x2f0 > erofs_fiemap+0x64/0x90 [erofs] > do_vfs_ioctl+0x40d/0x12e0 > __x64_sys_ioctl+0xaa/0x1c0 > do_syscall_64+0x35/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > </TASK> > ---[ end trace 0000000000000000 ]--- > watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] > > Reported-by: Hulk Robot <hulkci@huawei.com> > Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> > --- > fs/ioctl.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/fs/ioctl.c b/fs/ioctl.c > index 1ed097e94af2..7f70e90766ed 100644 > --- a/fs/ioctl.c > +++ b/fs/ioctl.c > @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > u32 incompat_flags; > int ret = 0; > > - if (*len == 0) > - return -EINVAL; > if (start > maxbytes) > return -EFBIG; > > @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > if (*len > maxbytes || (maxbytes - *len) < start) > *len = maxbytes - start; > > + if (*len == 0) > + return -EINVAL; Looks fine to me (and I don't even really mind pulling this in) but this isn't a patch to fs/iomap/ -- doesn't the same issue potentially affect the fiemap implementations that do not use iomap? Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > + > supported_flags |= FIEMAP_FLAG_SYNC; > supported_flags &= FIEMAP_FLAGS_COMPAT; > incompat_flags = fieinfo->fi_flags & ~supported_flags; > -- > 2.22.0 >
Hi Darrick, 在 2022/3/18 6:05, Darrick J. Wong wrote: > On Tue, Mar 15, 2022 at 02:57:45PM +0800, Guo Xuenan wrote: >> when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start >> will always true , then *len set zero. because of start offset is byhond >> file size, for erofs filesystem it will always return iomap.length with >> zero,iomap iterate will be infinite loop. >> >> In order to avoid this situation, it is better to calculate the actual >> mapping length at first. If the len is 0, there is no need to continue >> the operation. >> >> ------------[ cut here ]------------ >> WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 >> Modules linked in: xfs erofs >> CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 >> RIP: 0010:iomap_iter+0x97f/0xc70 >> Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be >> RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 >> RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 >> RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 >> RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 >> R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff >> R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 >> FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Call Trace: >> <TASK> >> iomap_fiemap+0x1c9/0x2f0 >> erofs_fiemap+0x64/0x90 [erofs] >> do_vfs_ioctl+0x40d/0x12e0 >> __x64_sys_ioctl+0xaa/0x1c0 >> do_syscall_64+0x35/0x80 >> entry_SYSCALL_64_after_hwframe+0x44/0xae >> </TASK> >> ---[ end trace 0000000000000000 ]--- >> watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] >> >> Reported-by: Hulk Robot <hulkci@huawei.com> >> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> >> --- >> fs/ioctl.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/ioctl.c b/fs/ioctl.c >> index 1ed097e94af2..7f70e90766ed 100644 >> --- a/fs/ioctl.c >> +++ b/fs/ioctl.c >> @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, >> u32 incompat_flags; >> int ret = 0; >> >> - if (*len == 0) >> - return -EINVAL; >> if (start > maxbytes) >> return -EFBIG; >> >> @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, >> if (*len > maxbytes || (maxbytes - *len) < start) >> *len = maxbytes - start; >> >> + if (*len == 0) >> + return -EINVAL; > Looks fine to me (and I don't even really mind pulling this in) but this > isn't a patch to fs/iomap/ -- doesn't the same issue potentially affect > the fiemap implementations that do not use iomap? > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --D Thanks Darrick, you're right,there is something wrong with my statement. In a strict sense, this modification here does not belong to fs/iomap, i can change it to fs/vfs in v2 :) I have looked into the code of those filesystem(btrfs,ext4,f2fs,nilfs2,ntfs3..) which don't use iomap, and did some test. when start=0x7fffffffffffffff, and len = 0; btrfs: while len==0, return -EINVAL directly; ext4: ext4_get_es_cache->ext4_fiemap_check_ranges, return -EFBIG; f2fs: return -EFBIG; nilfs2: while len==0, do nothing and return 0; ntfs3: return -EFBIG directly; so, as far as i can see, just return -EINVAL earlyier in fiemap_prep has no side effect. Thanks. >> + >> supported_flags |= FIEMAP_FLAG_SYNC; >> supported_flags &= FIEMAP_FLAGS_COMPAT; >> incompat_flags = fieinfo->fi_flags & ~supported_flags; >> -- >> 2.22.0 >> > .
On Fri, Mar 18, 2022 at 03:21:23PM +0800, Guo Xuenan wrote: > Hi Darrick, > > 在 2022/3/18 6:05, Darrick J. Wong wrote: > > On Tue, Mar 15, 2022 at 02:57:45PM +0800, Guo Xuenan wrote: > > > when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start > > > will always true , then *len set zero. because of start offset is byhond > > > file size, for erofs filesystem it will always return iomap.length with > > > zero,iomap iterate will be infinite loop. > > > > > > In order to avoid this situation, it is better to calculate the actual > > > mapping length at first. If the len is 0, there is no need to continue > > > the operation. > > > > > > ------------[ cut here ]------------ > > > WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 > > > Modules linked in: xfs erofs > > > CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 > > > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 > > > RIP: 0010:iomap_iter+0x97f/0xc70 > > > Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be > > > RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 > > > RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 > > > RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 > > > RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 > > > R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff > > > R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 > > > FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 > > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > > CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 > > > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > > > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > > > Call Trace: > > > <TASK> > > > iomap_fiemap+0x1c9/0x2f0 > > > erofs_fiemap+0x64/0x90 [erofs] > > > do_vfs_ioctl+0x40d/0x12e0 > > > __x64_sys_ioctl+0xaa/0x1c0 > > > do_syscall_64+0x35/0x80 > > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > > </TASK> > > > ---[ end trace 0000000000000000 ]--- > > > watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] > > > > > > Reported-by: Hulk Robot <hulkci@huawei.com> > > > Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> > > > --- > > > fs/ioctl.c | 5 +++-- > > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > > > diff --git a/fs/ioctl.c b/fs/ioctl.c > > > index 1ed097e94af2..7f70e90766ed 100644 > > > --- a/fs/ioctl.c > > > +++ b/fs/ioctl.c > > > @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > > > u32 incompat_flags; > > > int ret = 0; > > > - if (*len == 0) > > > - return -EINVAL; > > > if (start > maxbytes) > > > return -EFBIG; > > > @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, > > > if (*len > maxbytes || (maxbytes - *len) < start) > > > *len = maxbytes - start; > > > + if (*len == 0) > > > + return -EINVAL; > > Looks fine to me (and I don't even really mind pulling this in) but this > > isn't a patch to fs/iomap/ -- doesn't the same issue potentially affect > > the fiemap implementations that do not use iomap? > > > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > > > --D > > Thanks Darrick, you're right,there is something wrong with my statement. In > a strict sense, this modification here does not belong to fs/iomap, i can > change it to fs/vfs in v2 :) I have looked into the code of those > filesystem(btrfs,ext4,f2fs,nilfs2,ntfs3..) which don't use iomap, and did > some test. when start=0x7fffffffffffffff, and len = 0; btrfs: while len==0, > return -EINVAL directly; ext4: ext4_get_es_cache->ext4_fiemap_check_ranges, > return -EFBIG; f2fs: return -EFBIG; nilfs2: while len==0, do nothing and > return 0; ntfs3: return -EFBIG directly; so, as far as i can see, just > return -EINVAL earlyier in fiemap_prep has no side effect. It's dangerous for patch reviewers to think more about patches. :) But-- thinking further, why do we return EINVAL for a query length of zero? Why doesn't FIEMAP set fi_extents_mapped = 0 and return immediately? Oh, right, because the documentation (a) doesn't say much about return codes and (b) the current implementation returns *some* error code (EINVAL or EFBIG), so now people probably expect that. That said ... I think "File too large" is a more appropriate message than "Invalid argument" for when we truncated the request to maxbytes but then ended up with a zero-length request. Does changing the check at the top of the function to: if (*len == 0) return -EINVAL; if (start >= maxbytes) return -EFBIG; Cover this infinite loop case? (Admittedly, it is Sunday morning and the parts of my brain that handle integer rollover issues are still asleep.) --D > > Thanks. > > > > + > > > supported_flags |= FIEMAP_FLAG_SYNC; > > > supported_flags &= FIEMAP_FLAGS_COMPAT; > > > incompat_flags = fieinfo->fi_flags & ~supported_flags; > > > -- > > > 2.22.0 > > > > > .
diff --git a/fs/ioctl.c b/fs/ioctl.c index 1ed097e94af2..7f70e90766ed 100644 --- a/fs/ioctl.c +++ b/fs/ioctl.c @@ -171,8 +171,6 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, u32 incompat_flags; int ret = 0; - if (*len == 0) - return -EINVAL; if (start > maxbytes) return -EFBIG; @@ -182,6 +180,9 @@ int fiemap_prep(struct inode *inode, struct fiemap_extent_info *fieinfo, if (*len > maxbytes || (maxbytes - *len) < start) *len = maxbytes - start; + if (*len == 0) + return -EINVAL; + supported_flags |= FIEMAP_FLAG_SYNC; supported_flags &= FIEMAP_FLAGS_COMPAT; incompat_flags = fieinfo->fi_flags & ~supported_flags;
when get fiemap starting from MAX_LFS_FILESIZE, (maxbytes - *len) < start will always true , then *len set zero. because of start offset is byhond file size, for erofs filesystem it will always return iomap.length with zero,iomap iterate will be infinite loop. In order to avoid this situation, it is better to calculate the actual mapping length at first. If the len is 0, there is no need to continue the operation. ------------[ cut here ]------------ WARNING: CPU: 7 PID: 905 at fs/iomap/iter.c:35 iomap_iter+0x97f/0xc70 Modules linked in: xfs erofs CPU: 7 PID: 905 Comm: iomap Tainted: G W 5.17.0-rc8 #27 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 RIP: 0010:iomap_iter+0x97f/0xc70 Code: 85 a1 fc ff ff e8 71 be 9c ff 0f 1f 44 00 00 e9 92 fc ff ff e8 62 be 9c ff 0f 0b b8 fb ff ff ff e9 fc f8 ff ff e8 51 be 9c ff <0f> 0b e9 2b fc ff ff e8 45 be 9c ff 0f 0b e9 e1 fb ff ff e8 39 be RSP: 0018:ffff888060a37ab0 EFLAGS: 00010293 RAX: 0000000000000000 RBX: ffff888060a37bb0 RCX: 0000000000000000 RDX: ffff88807e19a900 RSI: ffffffff81a7da7f RDI: ffff888060a37be0 RBP: 7fffffffffffffff R08: 0000000000000000 R09: ffff888060a37c20 R10: ffff888060a37c67 R11: ffffed100c146f8c R12: 7fffffffffffffff R13: 0000000000000000 R14: ffff888060a37bd8 R15: ffff888060a37c20 FS: 00007fd3cca01540(0000) GS:ffff888108780000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000020010820 CR3: 0000000054b92000 CR4: 00000000000006e0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> iomap_fiemap+0x1c9/0x2f0 erofs_fiemap+0x64/0x90 [erofs] do_vfs_ioctl+0x40d/0x12e0 __x64_sys_ioctl+0xaa/0x1c0 do_syscall_64+0x35/0x80 entry_SYSCALL_64_after_hwframe+0x44/0xae </TASK> ---[ end trace 0000000000000000 ]--- watchdog: BUG: soft lockup - CPU#7 stuck for 26s! [iomap:905] Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: Guo Xuenan <guoxuenan@huawei.com> --- fs/ioctl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)