All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laura Abbott <labbott@redhat.com>
To: Al Viro <viro@ZenIV.linux.org.uk>, David Howells <dhowells@redhat.com>
Cc: Laura Abbott <labbott@redhat.com>,
	Jeremi Piotrowski <jeremi.piotrowski@gmail.com>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Phillip Lougher <phillip@squashfs.org.uk>,
	linux-kernel@vger.kernel.org, Ilya Dryomov <idryomov@gmail.com>
Subject: [PATCHv2] vfs: Handle file systems without ->parse_params better
Date: Thu, 12 Dec 2019 17:41:39 -0500	[thread overview]
Message-ID: <20191212224139.15970-1-labbott@redhat.com> (raw)


The new mount API relies on file systems to provide a ->parse_params
function to handle parsing of arguments. If a file system doesn't
have a ->parse_param function, it falls back to parsing the source
option and rejecting all other options. This is a change in behavior
for some file systems which would just quietly ignore extra options
and mount successfully. This was noticed by users as squashfs failing
to mount with extra options after the conversion to the new mount
API.

File systems with a ->parse_params function rely on the top level
to parse the "source" param so we can't easily move that around. To
get around this, introduce a default parsing functions for file
systems that take no arguments. This parses only the "source" option
and only logs an error for other arguments. Update the comment
to reflect this expected behavior for "source" parsing as well.

Fixes: 3e1aeb00e6d1 ("vfs: Implement a filesystem superblock
creation/configuration context")
Link: https://lore.kernel.org/lkml/20191130181548.GA28459@gentoo-tp.home/
Reported-by: Jeremi Piotrowski <jeremi.piotrowski@gmail.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1781863
Signed-off-by: Laura Abbott <labbott@redhat.com>
---
v2: Dropped most the boiler plate for parsing and just compared
against "source". Renamed to ignore_unknown_parse_param.
---
 fs/fs_context.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/fs/fs_context.c b/fs/fs_context.c
index 138b5b4d621d..086ade29b811 100644
--- a/fs/fs_context.c
+++ b/fs/fs_context.c
@@ -107,6 +107,22 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
 	return -ENOPARAM;
 }
 
+/**
+ * ignore_unknowns_parse_param - ->parse_param function for a file system that
+ * takes no arguments
+ * @fc: The filesystem context
+ * @param: The parameter.
+ */
+static int ignore_unknown_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+
+	if (strcmp(param->key, "source") == 0)
+		return -ENOPARAM;
+	/* Just log an error for backwards compatibility */
+	errorf(fc, "%s: Unknown parameter '%s'", fc->fs_type->name, param->key);
+	return 0;
+}
+
 /**
  * vfs_parse_fs_param - Add a single parameter to a superblock config
  * @fc: The filesystem context to modify
@@ -126,6 +142,7 @@ static int vfs_parse_sb_flag(struct fs_context *fc, const char *key)
 int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
 {
 	int ret;
+	int (*parse_param)(struct fs_context *, struct fs_parameter *);
 
 	if (!param->key)
 		return invalf(fc, "Unnamed parameter\n");
@@ -141,14 +158,19 @@ int vfs_parse_fs_param(struct fs_context *fc, struct fs_parameter *param)
 		 */
 		return ret;
 
-	if (fc->ops->parse_param) {
-		ret = fc->ops->parse_param(fc, param);
-		if (ret != -ENOPARAM)
-			return ret;
-	}
+	parse_param = fc->ops->parse_param;
+	if (!parse_param)
+		parse_param = ignore_unknown_parse_param;
+
+	ret = parse_param(fc, param);
+	if (ret != -ENOPARAM)
+		return ret;
 
-	/* If the filesystem doesn't take any arguments, give it the
-	 * default handling of source.
+	/*
+	 * File systems may have a ->parse_param function but rely on
+	 * the top level to parse the source function. File systems
+	 * may have their own source parsing though so this needs
+	 * to come after the call to parse_param above.
 	 */
 	if (strcmp(param->key, "source") == 0) {
 		if (param->type != fs_value_is_string)
-- 
2.21.0


             reply	other threads:[~2019-12-12 22:41 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-12 22:41 Laura Abbott [this message]
2019-12-12 22:50 ` [PATCHv2] vfs: Handle file systems without ->parse_params better Randy Dunlap

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191212224139.15970-1-labbott@redhat.com \
    --to=labbott@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=idryomov@gmail.com \
    --cc=jeremi.piotrowski@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phillip@squashfs.org.uk \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.