Skip to contentSkip to conversation

vfs: fix FIGETBSZ ioctl on an overlayfs file

PATCH In review Amir Goldstein 2 participants 3 messages Oct 11, 2018 – Oct 13, 2018

Subsystems (2)
VFS Landlock security module
lore
Share & tools
lore

Amir Goldstein

author
#0 lore raw

PATCH v2 1/1 vfs: fix FIGETBSZ ioctl on an overlayfs file

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 <[email protected]>
---
 fs/ioctl.c | 3 +++
 1 file changed, 3 insertions(+)
fs/ioctl.c +3 -0
index 2005529af560..0400297c8d72 100644
@@ -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:
Signature · 1 line
2.17.1
maintainer
#1 lore raw
Quoted 18 lines · Amir Goldstein

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 <[email protected]>
---
 fs/ioctl.c | 3 +++
 1 file changed, 3 insertions(+)
fs/ioctl.c +0 -0
index 2005529af560..0400297c8d72 100644

fs/ioctl.c int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd, · line 675

Quoted 7 lines
fs/ioctl.c +3 -0
@@ -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?

author
#2 lore raw
Quoted 22 lines · Al Viro
Quoted 20 lines · Amir Goldstein

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 <[email protected]>
---
 fs/ioctl.c | 3 +++
 1 file changed, 3 insertions(+)
fs/ioctl.c +0 -0
index 2005529af560..0400297c8d72 100644
@@ -669,6 +669,9 @@ int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd,
              return ioctl_fiemap(filp, arg);
Quoted 8 lines

fs/ioctl.c int do_vfs_ioctl(struct file *filp, unsigned int fd, unsigned int cmd, · line 675

Quoted 5 lines
fs/ioctl.c +3 -0
      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 <[email protected]>
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 <[email protected]>
Signed-off-by Theodore Ts'o <[email protected]>