| Message ID | 20220725032050.3873372-1-lijinlin3@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 17d9c15c9b9e7fb285f7ac5367dfb5f00ff575e3 |
| Headers | show |
| Series | fsdax: Fix infinite loop in dax_iomap_rw() | expand |
On Mon, Jul 25, 2022 at 11:20:50AM +0800, Li Jinlin wrote: > I got an infinite loop and a WARNING report when executing a tail command > in virtiofs. > > WARNING: CPU: 10 PID: 964 at fs/iomap/iter.c:34 iomap_iter+0x3a2/0x3d0 > Modules linked in: > CPU: 10 PID: 964 Comm: tail Not tainted 5.19.0-rc7 > Call Trace: > <TASK> > dax_iomap_rw+0xea/0x620 > ? __this_cpu_preempt_check+0x13/0x20 > fuse_dax_read_iter+0x47/0x80 > fuse_file_read_iter+0xae/0xd0 > new_sync_read+0xfe/0x180 > ? 0xffffffff81000000 > vfs_read+0x14d/0x1a0 > ksys_read+0x6d/0xf0 > __x64_sys_read+0x1a/0x20 > do_syscall_64+0x3b/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > The tail command will call read() with a count of 0. In this case, > iomap_iter() will report this WARNING, and always return 1 which casuing > the infinite loop in dax_iomap_rw(). > > Fixing by checking count whether is 0 in dax_iomap_rw(). > > Fixes: ca289e0b95af ("fsdax: switch dax_iomap_rw to use iomap_iter") > Signed-off-by: Li Jinlin <lijinlin3@huawei.com> Huh, I didn't know FUSE supports DAX and iomap now... > --- > fs/dax.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/fs/dax.c b/fs/dax.c > index 4155a6107fa1..7ab248ed21aa 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -1241,6 +1241,9 @@ dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter, > loff_t done = 0; > int ret; > > + if (!iomi.len) > + return 0; Hmm, most of the callers of dax_iomap_rw skip the whole call if iov_iter_count(to)==0, so I wonder if fuse_dax_read_iter should do the same? That said, iomap_dio_rw bails early if you pass it iomi.len, so I don't have any real objections to this. Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > + > if (iov_iter_rw(iter) == WRITE) { > lockdep_assert_held_write(&iomi.inode->i_rwsem); > iomi.flags |= IOMAP_WRITE; > -- > 2.30.2 >
Darrick J. Wong wrote: > On Mon, Jul 25, 2022 at 11:20:50AM +0800, Li Jinlin wrote: > > I got an infinite loop and a WARNING report when executing a tail command > > in virtiofs. > > > > WARNING: CPU: 10 PID: 964 at fs/iomap/iter.c:34 iomap_iter+0x3a2/0x3d0 > > Modules linked in: > > CPU: 10 PID: 964 Comm: tail Not tainted 5.19.0-rc7 > > Call Trace: > > <TASK> > > dax_iomap_rw+0xea/0x620 > > ? __this_cpu_preempt_check+0x13/0x20 > > fuse_dax_read_iter+0x47/0x80 > > fuse_file_read_iter+0xae/0xd0 > > new_sync_read+0xfe/0x180 > > ? 0xffffffff81000000 > > vfs_read+0x14d/0x1a0 > > ksys_read+0x6d/0xf0 > > __x64_sys_read+0x1a/0x20 > > do_syscall_64+0x3b/0x90 > > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > > > The tail command will call read() with a count of 0. In this case, > > iomap_iter() will report this WARNING, and always return 1 which casuing > > the infinite loop in dax_iomap_rw(). > > > > Fixing by checking count whether is 0 in dax_iomap_rw(). > > > > Fixes: ca289e0b95af ("fsdax: switch dax_iomap_rw to use iomap_iter") > > Signed-off-by: Li Jinlin <lijinlin3@huawei.com> > > Huh, I didn't know FUSE supports DAX and iomap now... Yeah, it came in via DAX support for virtio-fs. > > --- > > fs/dax.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/fs/dax.c b/fs/dax.c > > index 4155a6107fa1..7ab248ed21aa 100644 > > --- a/fs/dax.c > > +++ b/fs/dax.c > > @@ -1241,6 +1241,9 @@ dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter, > > loff_t done = 0; > > int ret; > > > > + if (!iomi.len) > > + return 0; > > Hmm, most of the callers of dax_iomap_rw skip the whole call if > iov_iter_count(to)==0, so I wonder if fuse_dax_read_iter should do the > same? > > That said, iomap_dio_rw bails early if you pass it iomi.len, so I don't > have any real objections to this. That was the same conclusion I came to... > Reviewed-by: Darrick J. Wong <djwong@kernel.org> Thanks, will get this merged up for v5.19-final.
diff --git a/fs/dax.c b/fs/dax.c index 4155a6107fa1..7ab248ed21aa 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -1241,6 +1241,9 @@ dax_iomap_rw(struct kiocb *iocb, struct iov_iter *iter, loff_t done = 0; int ret; + if (!iomi.len) + return 0; + if (iov_iter_rw(iter) == WRITE) { lockdep_assert_held_write(&iomi.inode->i_rwsem); iomi.flags |= IOMAP_WRITE;
I got an infinite loop and a WARNING report when executing a tail command in virtiofs. WARNING: CPU: 10 PID: 964 at fs/iomap/iter.c:34 iomap_iter+0x3a2/0x3d0 Modules linked in: CPU: 10 PID: 964 Comm: tail Not tainted 5.19.0-rc7 Call Trace: <TASK> dax_iomap_rw+0xea/0x620 ? __this_cpu_preempt_check+0x13/0x20 fuse_dax_read_iter+0x47/0x80 fuse_file_read_iter+0xae/0xd0 new_sync_read+0xfe/0x180 ? 0xffffffff81000000 vfs_read+0x14d/0x1a0 ksys_read+0x6d/0xf0 __x64_sys_read+0x1a/0x20 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd The tail command will call read() with a count of 0. In this case, iomap_iter() will report this WARNING, and always return 1 which casuing the infinite loop in dax_iomap_rw(). Fixing by checking count whether is 0 in dax_iomap_rw(). Fixes: ca289e0b95af ("fsdax: switch dax_iomap_rw to use iomap_iter") Signed-off-by: Li Jinlin <lijinlin3@huawei.com> --- fs/dax.c | 3 +++ 1 file changed, 3 insertions(+)