* [PATCH v2] vfs: fix FIGETBSZ ioctl on an overlayfs file
@ 2018-10-11 14:38 Amir Goldstein
2018-10-13 4:10 ` Al Viro
0 siblings, 1 reply; 3+ messages in thread
From: Amir Goldstein @ 2018-10-11 14:38 UTC (permalink / raw)
To: Miklos Szeredi; +Cc: Al Viro, linux-unionfs, linux-fsdevel
Some anon_bdev filesystems (e.g. overlayfs, ceph) don't have s_blocksize
set. Returning zero from FIGETBSZ ioctl results in a Floating point
exception from the e2fsprogs utility filefrag, which divides the size of
the file with the value returned by FIGETBSZ.
Fix the interface by returning -EINVAL for these filesystems.
Fixes: d1d04ef8572b ("ovl: stack file ops")
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
fs/ioctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 2005529af560..0400297c8d72 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -669,6 +669,9 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
return ioctl_fiemap(filp, arg);
case FIGETBSZ:
+ /* anon_bdev filesystems may not have a block size */
+ if (!inode->i_sb->s_blocksize)
+ return -EINVAL;
return put_user(inode->i_sb->s_blocksize, argp);
case FICLONE:
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vfs: fix FIGETBSZ ioctl on an overlayfs file
2018-10-11 14:38 [PATCH v2] vfs: fix FIGETBSZ ioctl on an overlayfs file Amir Goldstein
@ 2018-10-13 4:10 ` Al Viro
2018-10-13 7:33 ` Amir Goldstein
0 siblings, 1 reply; 3+ messages in thread
From: Al Viro @ 2018-10-13 4:10 UTC (permalink / raw)
To: Amir Goldstein; +Cc: Miklos Szeredi, linux-unionfs, linux-fsdevel
On Thu, Oct 11, 2018 at 05:38:14PM +0300, Amir Goldstein wrote:
> Some anon_bdev filesystems (e.g. overlayfs, ceph) don't have s_blocksize
> set. Returning zero from FIGETBSZ ioctl results in a Floating point
> exception from the e2fsprogs utility filefrag, which divides the size of
> the file with the value returned by FIGETBSZ.
>
> Fix the interface by returning -EINVAL for these filesystems.
>
> Fixes: d1d04ef8572b ("ovl: stack file ops")
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
> fs/ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 2005529af560..0400297c8d72 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -669,6 +669,9 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
> return ioctl_fiemap(filp, arg);
>
> case FIGETBSZ:
> + /* anon_bdev filesystems may not have a block size */
> + if (!inode->i_sb->s_blocksize)
> + return -EINVAL;
> return put_user(inode->i_sb->s_blocksize, argp);
Probably makes sense... Out of curiosity - what does the same utility do
when faced with -EINVAL here?
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vfs: fix FIGETBSZ ioctl on an overlayfs file
2018-10-13 4:10 ` Al Viro
@ 2018-10-13 7:33 ` Amir Goldstein
0 siblings, 0 replies; 3+ messages in thread
From: Amir Goldstein @ 2018-10-13 7:33 UTC (permalink / raw)
To: Al Viro; +Cc: Miklos Szeredi, overlayfs, linux-fsdevel, Theodore Tso
On Sat, Oct 13, 2018 at 7:10 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Thu, Oct 11, 2018 at 05:38:14PM +0300, Amir Goldstein wrote:
> > Some anon_bdev filesystems (e.g. overlayfs, ceph) don't have s_blocksize
> > set. Returning zero from FIGETBSZ ioctl results in a Floating point
> > exception from the e2fsprogs utility filefrag, which divides the size of
> > the file with the value returned by FIGETBSZ.
> >
> > Fix the interface by returning -EINVAL for these filesystems.
> >
> > Fixes: d1d04ef8572b ("ovl: stack file ops")
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> > fs/ioctl.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/ioctl.c b/fs/ioctl.c
> > index 2005529af560..0400297c8d72 100644
> > --- a/fs/ioctl.c
> > +++ b/fs/ioctl.c
> > @@ -669,6 +669,9 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
> > return ioctl_fiemap(filp, arg);
> >
> > case FIGETBSZ:
> > + /* anon_bdev filesystems may not have a block size */
> > + if (!inode->i_sb->s_blocksize)
> > + return -EINVAL;
> > return put_user(inode->i_sb->s_blocksize, argp);
>
> Probably makes sense... Out of curiosity - what does the same utility do
> when faced with -EINVAL here?
filefrag was actually converted from FIBMAP/FIGETBSZ to FIEMAP/statfs(2)
but apparently that didn't work well for some fs, so FIGETBSZ was
brought back from retirement to override b_size.
When ioctl(FIGETBSZ) fails filefrag continues with f_bsize without any
complains.
FYI, another utility in e2fsprogs that uses FIGETBSZ, copy_sparse, will
simply exit when ioctl(FIGETBSZ) fails. But copy_sparse only has
FIBMAP implementation so it is going to fail on overlay file anyway.
Cheers,
Amir.
commit 01824c9bbcd2c1037085213f6cd7d591f78db9c5
Author: Rakesh Pandit <rakesh@tuxera.com>
Date: Sun Jul 27 20:04:48 2014 -0400
filefrag: fix block size value
ioctl(FIGETBSZ) was used to get block size earlier but 2508eaa7
(filefrag: improvements to filefrag FIEMAP handling) moved to fstatfs
f_bsize which doesn't work well for many files systems.
Block size returned using fstatfs isn't block size but "optimal
transfer block size" as per man page. Even stat st_blksize is
"preferred I/O block size" and in may file systems it may even vary
from file to file (POSIX). This patch changes filefrag to use
FIGETBSZ preferentially over f_bsize.
[ Modified by tytso to add the fallback to f_bsize if FIGETBSZ fails
for some reason ]
Signed-off-by: Rakesh Pandit <rakesh@tuxera.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-10-13 15:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-11 14:38 [PATCH v2] vfs: fix FIGETBSZ ioctl on an overlayfs file Amir Goldstein
2018-10-13 4:10 ` Al Viro
2018-10-13 7:33 ` Amir Goldstein
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).