Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/3] fuse: use ->reconfigure() instead of ->remount_fs()
@ 2020-07-10 11:58 Miklos Szeredi
  2020-07-10 11:58 ` [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT) Miklos Szeredi
  2020-07-10 11:58 ` [PATCH 3/3] fuse: reject options on reconfigure via fsconfig(2) Miklos Szeredi
  0 siblings, 2 replies; 5+ messages in thread
From: Miklos Szeredi @ 2020-07-10 11:58 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Stefan Priebe, David Howells, stable

s_op->remount_fs() is only called from legacy_reconfigure(), which is not
used after being converted to the new API.

Convert to using ->reconfigure().  This restores the previous behavior of
syncing the filesystem and rejecting MS_MANDLOCK on remount.

Fixes: c30da2e981a7 ("fuse: convert to use the new mount API")
Cc: <stable@vger.kernel.org> # v5.4
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/inode.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 5b4aebf5821f..be39dff57c28 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -121,10 +121,12 @@ static void fuse_evict_inode(struct inode *inode)
 	}
 }
 
-static int fuse_remount_fs(struct super_block *sb, int *flags, char *data)
+static int fuse_reconfigure(struct fs_context *fc)
 {
+	struct super_block *sb = fc->root->d_sb;
+
 	sync_filesystem(sb);
-	if (*flags & SB_MANDLOCK)
+	if (fc->sb_flags & SB_MANDLOCK)
 		return -EINVAL;
 
 	return 0;
@@ -817,7 +819,6 @@ static const struct super_operations fuse_super_operations = {
 	.evict_inode	= fuse_evict_inode,
 	.write_inode	= fuse_write_inode,
 	.drop_inode	= generic_delete_inode,
-	.remount_fs	= fuse_remount_fs,
 	.put_super	= fuse_put_super,
 	.umount_begin	= fuse_umount_begin,
 	.statfs		= fuse_statfs,
@@ -1296,6 +1297,7 @@ static int fuse_get_tree(struct fs_context *fc)
 static const struct fs_context_operations fuse_context_ops = {
 	.free		= fuse_free_fc,
 	.parse_param	= fuse_parse_param,
+	.reconfigure	= fuse_reconfigure,
 	.get_tree	= fuse_get_tree,
 };
 
-- 
2.21.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT)
  2020-07-10 11:58 [PATCH 1/3] fuse: use ->reconfigure() instead of ->remount_fs() Miklos Szeredi
@ 2020-07-10 11:58 ` Miklos Szeredi
  2020-07-16  0:27   ` Sasha Levin
  2020-07-10 11:58 ` [PATCH 3/3] fuse: reject options on reconfigure via fsconfig(2) Miklos Szeredi
  1 sibling, 1 reply; 5+ messages in thread
From: Miklos Szeredi @ 2020-07-10 11:58 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Stefan Priebe, David Howells, stable

The command

  mount -o remount -o unknownoption /mnt/fuse

succeeds on kernel versions prior to v5.4 and fails on kernel version at or
after.  This is because fuse_parse_param() rejects any unrecognised options
in case of FS_CONTEXT_FOR_RECONFIGURE, just as for FS_CONTEXT_FOR_MOUNT.

This causes a regression in case the fuse filesystem is in fstab, since
remount sends all options found there to the kernel; even ones that are
meant for the initial mount and are consumed by the userspace fuse server.

Fix this by ignoring mount options, just as fuse_remount_fs() did prior to
the conversion to the new API.

Reported-by: Stefan Priebe <s.priebe@profihost.ag>
Fixes: c30da2e981a7 ("fuse: convert to use the new mount API")
Cc: <stable@vger.kernel.org> # v5.4
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/inode.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index be39dff57c28..ba201bf5ffad 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -477,6 +477,13 @@ static int fuse_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct fuse_fs_context *ctx = fc->fs_private;
 	int opt;
 
+	/*
+	 * Ignore options coming from mount(MS_REMOUNT) for backward
+	 * compatibility.
+	 */
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+		return 0;
+
 	opt = fs_parse(fc, fuse_fs_parameters, param, &result);
 	if (opt < 0)
 		return opt;
-- 
2.21.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 3/3] fuse: reject options on reconfigure via fsconfig(2)
  2020-07-10 11:58 [PATCH 1/3] fuse: use ->reconfigure() instead of ->remount_fs() Miklos Szeredi
  2020-07-10 11:58 ` [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT) Miklos Szeredi
@ 2020-07-10 11:58 ` Miklos Szeredi
  1 sibling, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2020-07-10 11:58 UTC (permalink / raw)
  To: linux-fsdevel; +Cc: Stefan Priebe, David Howells

Previous patch changed handling of remount/reconfigure to ignore all
options, including those that are unknown to the fuse kernel fs.  This was
done for backward compatibility, but this likely only affects the old
mount(2) API.

The new fsconfig(2) based reconfiguration could possibly be improved.  This
would make the new API less of a drop in replacement for the old, OTOH this
is a good chance to get rid of some weirdnesses in the old API.

Several other behaviors might make sense:

 1) unknown options are rejected, known options are ignored

 2) unknown options are rejected, known options are rejected if the value
 is changed, allowed otherwise

 3) all options are rejected

Prior to the backward compatibility fix to ignore all options all known
options were accepted (1), even if they change the value of a mount
parameter; fuse_reconfigure() does not look at the config values set by
fuse_parse_param().

To fix that we'd need to verify that the value provided is the same as set
in the initial configuration (2).  The major drawback is that this is much
more complex than just rejecting all attempts at changing options (3);
i.e. all options signify initial configuration values and don't make sense
on reconfigure.

This patch opts for (3) with the rationale that no mount options are
reconfigurable in fuse.

Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
---
 fs/fuse/inode.c            | 16 ++++++++++------
 fs/namespace.c             |  1 +
 include/linux/fs_context.h |  1 +
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index ba201bf5ffad..bba747520e9b 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -477,12 +477,16 @@ static int fuse_parse_param(struct fs_context *fc, struct fs_parameter *param)
 	struct fuse_fs_context *ctx = fc->fs_private;
 	int opt;
 
-	/*
-	 * Ignore options coming from mount(MS_REMOUNT) for backward
-	 * compatibility.
-	 */
-	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
-		return 0;
+	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
+		/*
+		 * Ignore options coming from mount(MS_REMOUNT) for backward
+		 * compatibility.
+		 */
+		if (fc->oldapi)
+			return 0;
+
+		return invalfc(fc, "No changes allowed in reconfigure");
+	}
 
 	opt = fs_parse(fc, fuse_fs_parameters, param, &result);
 	if (opt < 0)
diff --git a/fs/namespace.c b/fs/namespace.c
index f30ed401cc6d..4a0f600a3328 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2603,6 +2603,7 @@ static int do_remount(struct path *path, int ms_flags, int sb_flags,
 	if (IS_ERR(fc))
 		return PTR_ERR(fc);
 
+	fc->oldapi = true;
 	err = parse_monolithic_mount_data(fc, data);
 	if (!err) {
 		down_write(&sb->s_umount);
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 5f24fcbfbfb4..37e1e8f7f08d 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -109,6 +109,7 @@ struct fs_context {
 	enum fs_context_phase	phase:8;	/* The phase the context is in */
 	bool			need_free:1;	/* Need to call ops->free() */
 	bool			global:1;	/* Goes into &init_user_ns */
+	bool			oldapi:1;	/* Coming from mount(2) */
 };
 
 struct fs_context_operations {
-- 
2.21.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT)
  2020-07-10 11:58 ` [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT) Miklos Szeredi
@ 2020-07-16  0:27   ` Sasha Levin
  2020-07-20  9:14     ` Miklos Szeredi
  0 siblings, 1 reply; 5+ messages in thread
From: Sasha Levin @ 2020-07-16  0:27 UTC (permalink / raw)
  To: Sasha Levin, Miklos Szeredi, linux-fsdevel; +Cc: Stefan Priebe, stable

Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: c30da2e981a7 ("fuse: convert to use the new mount API").

The bot has tested the following trees: v5.7.8, v5.4.51.

v5.7.8: Build OK!
v5.4.51: Failed to apply! Possible dependencies:
    7f5d38141e309 ("new primitive: __fs_parse()")
    82995cc6c5ae4 ("libceph, rbd, ceph: convert to use the new mount API")
    d7167b149943e ("fs_parse: fold fs_parameter_desc/fs_parameter_spec")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT)
  2020-07-16  0:27   ` Sasha Levin
@ 2020-07-20  9:14     ` Miklos Szeredi
  0 siblings, 0 replies; 5+ messages in thread
From: Miklos Szeredi @ 2020-07-20  9:14 UTC (permalink / raw)
  To: Sasha Levin; +Cc: Miklos Szeredi, linux-fsdevel, Stefan Priebe, stable

[-- Attachment #1: Type: text/plain, Size: 764 bytes --]

On Thu, Jul 16, 2020 at 2:27 AM Sasha Levin <sashal@kernel.org> wrote:
>
> Hi
>
> [This is an automated email]
>
> This commit has been processed because it contains a "Fixes:" tag
> fixing commit: c30da2e981a7 ("fuse: convert to use the new mount API").
>
> The bot has tested the following trees: v5.7.8, v5.4.51.
>
> v5.7.8: Build OK!
> v5.4.51: Failed to apply! Possible dependencies:
>     7f5d38141e309 ("new primitive: __fs_parse()")
>     82995cc6c5ae4 ("libceph, rbd, ceph: convert to use the new mount API")
>     d7167b149943e ("fs_parse: fold fs_parameter_desc/fs_parameter_spec")
>
>
> NOTE: The patch will not be queued to stable trees until it is upstream.
>
> How should we proceed with this patch?

Conflict resolution is trivial.

Thanks,
Miklos

[-- Attachment #2: merge.patch --]
[-- Type: text/x-patch, Size: 570 bytes --]

diff --cc fs/fuse/inode.c
index 16aec32f7f3d,ba201bf5ffad..000000000000
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@@ -473,7 -477,14 +473,14 @@@ static int fuse_parse_param(struct fs_c
  	struct fuse_fs_context *ctx = fc->fs_private;
  	int opt;
  
+ 	/*
+ 	 * Ignore options coming from mount(MS_REMOUNT) for backward
+ 	 * compatibility.
+ 	 */
+ 	if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE)
+ 		return 0;
+ 
 -	opt = fs_parse(fc, fuse_fs_parameters, param, &result);
 +	opt = fs_parse(fc, &fuse_fs_parameters, param, &result);
  	if (opt < 0)
  		return opt;
  

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-20  9:14 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-10 11:58 [PATCH 1/3] fuse: use ->reconfigure() instead of ->remount_fs() Miklos Szeredi
2020-07-10 11:58 ` [PATCH 2/3] fuse: ignore 'data' argument of mount(..., MS_REMOUNT) Miklos Szeredi
2020-07-16  0:27   ` Sasha Levin
2020-07-20  9:14     ` Miklos Szeredi
2020-07-10 11:58 ` [PATCH 3/3] fuse: reject options on reconfigure via fsconfig(2) Miklos Szeredi

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).