[rh7] fuse: don't declare FUSE FIEMAP support for generic FUSE fs

Submitted by Konstantin Khorenko on Dec. 12, 2019, 3:55 p.m.

Details

Message ID 20191212155545.10695-1-khorenko@virtuozzo.com
State New
Series "fuse: don't declare FUSE FIEMAP support for generic FUSE fs"
Headers show

Commit Message

Konstantin Khorenko Dec. 12, 2019, 3:55 p.m.
Stock kernels don't have FIEMAP support in fuse filesystems,
we've added the support for FIEMAP for pStorage performance.

Now if you call FIEMAP ioctl on a FUSE fs (for example mergerfs, sshfs,
mhddfs), libfuse crashes (the function fuse_lib_ioctl aborts due to
inbufsz > != outbufsz).

Let's pretend we don't provide FIEMAP for any FUSE fs except for pStorage.

https://bugs.openvz.org/browse/OVZ-7145

Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/ioctl.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/fs/ioctl.c b/fs/ioctl.c
index 0eeebd1723aaa..99a5df85306cf 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -180,7 +180,12 @@  static int ioctl_fiemap(struct file *filp, unsigned long arg)
 	u64 len;
 	int error;
 
-	if (!inode->i_op->fiemap)
+	/*
+	 * Among FUSE filesystems only pStorage has FIEMAP support,
+	 * generic libfuse is not ready for it.
+	 */
+	if ((!inode->i_op->fiemap) ||
+	    ((sb->s_magic == FUSE_SUPER_MAGIC) && !IS_PSTORAGE(sb)))
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))

Comments

Kirill Tkhai Dec. 12, 2019, 4:11 p.m.
On 12.12.2019 18:55, Konstantin Khorenko wrote:
> Stock kernels don't have FIEMAP support in fuse filesystems,
> we've added the support for FIEMAP for pStorage performance.
> 
> Now if you call FIEMAP ioctl on a FUSE fs (for example mergerfs, sshfs,
> mhddfs), libfuse crashes (the function fuse_lib_ioctl aborts due to
> inbufsz > != outbufsz).
> 
> Let's pretend we don't provide FIEMAP for any FUSE fs except for pStorage.
> 
> https://bugs.openvz.org/browse/OVZ-7145
> 
> Signed-off-by: Konstantin Khorenko <khorenko@virtuozzo.com>
> ---
>  fs/ioctl.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 0eeebd1723aaa..99a5df85306cf 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -180,7 +180,12 @@ static int ioctl_fiemap(struct file *filp, unsigned long arg)
>  	u64 len;
>  	int error;
>  
> -	if (!inode->i_op->fiemap)
> +	/*
> +	 * Among FUSE filesystems only pStorage has FIEMAP support,
> +	 * generic libfuse is not ready for it.
> +	 */
> +	if ((!inode->i_op->fiemap) ||
> +	    ((sb->s_magic == FUSE_SUPER_MAGIC) && !IS_PSTORAGE(sb)))
>  		return -EOPNOTSUPP;

1)This is generic fiemap() function, and it's not good to overload it with
a knowledge of vstorage. One more thing is it is called every time, when
fiemap is called (at least for fuse).

I think better place is fuse_mount(). We may set fc->no_fiemap = 1 there
unless there is a vstorage mount.

2)The best way would be make fc->no_fiemap autoconfigurable. See fuse_fiemap():
it already tries to do that. In case of userspace returns -ENOSYS, the function
disables fiemap: fc->no_fiemap = 1.

The problem is userspace crashes on first call of this command, isn't it?!
Can we make the first call, which never fails? Say, something like to do a fake
call of this ioctl with fieinfo->fi_extents_max = 0. Will userspace crash after
this too?

Kirill