[PATCH 0/8] autofs - convert to to use mount api

[PATCH 0/8] autofs - convert to to use mount api

[Ian Kent]

There was a patch from David Howells to convert autofs to use the mount
api but it was never merged.

I have taken David's patch and refactored it to make the change easier
to review in the hope of having it merged.

Signed-off-by: Ian Kent <raven@themaw.net>

Ian Kent (8):
  autofs: refactor autofs_prepare_pipe()
  autofs: add autofs_parse_fd()
  autofs: refactor super block info init
  autofs: reformat 0pt enum declaration
  autofs: refactor parse_options()
  autofs: validate protocol version
  autofs: convert autofs to use the new mount api
  autofs: fix protocol sub version setting

 fs/autofs/autofs_i.h |  15 +-
 fs/autofs/init.c     |   9 +-
 fs/autofs/inode.c    | 423 +++++++++++++++++++++++++------------------
 3 files changed, 266 insertions(+), 181 deletions(-)

-- 
2.41.0

[PATCH 1/8] autofs: refactor autofs_prepare_pipe()

[Ian Kent]

Refactor autofs_prepare_pipe() by seperating out a check function
to be used later.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/autofs_i.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index d5a44fa88acf..c24d32be7937 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -209,12 +209,20 @@ int autofs_fill_super(struct super_block *, void *, int);
 struct autofs_info *autofs_new_ino(struct autofs_sb_info *);
 void autofs_clean_ino(struct autofs_info *);
 
-static inline int autofs_prepare_pipe(struct file *pipe)
+static inline int autofs_check_pipe(struct file *pipe)
 {
 	if (!(pipe->f_mode & FMODE_CAN_WRITE))
 		return -EINVAL;
 	if (!S_ISFIFO(file_inode(pipe)->i_mode))
 		return -EINVAL;
+	return 0;
+}
+
+static inline int autofs_prepare_pipe(struct file *pipe)
+{
+	int ret = autofs_check_pipe(pipe);
+	if (ret < 0)
+		return ret;
 	/* We want a packet pipe */
 	pipe->f_flags |= O_DIRECT;
 	/* We don't expect -EAGAIN */
-- 
2.41.0

[PATCH 2/8] autofs: add autofs_parse_fd()

[Ian Kent]

Factor out the fd mount option handling.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 48 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 2b49662ed237..e279e275b0a5 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -129,6 +129,33 @@ static const match_table_t tokens = {
 	{Opt_err, NULL}
 };
 
+static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
+{
+	struct file *pipe;
+	int ret;
+
+	pipe = fget(fd);
+	if (!pipe) {
+		pr_err("could not open pipe file descriptor\n");
+		return -EBADF;
+	}
+
+	ret = autofs_check_pipe(pipe);
+	if (ret < 0) {
+		pr_err("Invalid/unusable pipe\n");
+		fput(pipe);
+		return -EBADF;
+	}
+
+	if (sbi->pipe)
+		fput(sbi->pipe);
+
+	sbi->pipefd = fd;
+	sbi->pipe = pipe;
+
+	return 0;
+}
+
 static int parse_options(char *options,
 			 struct inode *root, int *pgrp, bool *pgrp_set,
 			 struct autofs_sb_info *sbi)
@@ -139,6 +166,7 @@ static int parse_options(char *options,
 	int pipefd = -1;
 	kuid_t uid;
 	kgid_t gid;
+	int ret;
 
 	root->i_uid = current_uid();
 	root->i_gid = current_gid();
@@ -162,7 +190,9 @@ static int parse_options(char *options,
 		case Opt_fd:
 			if (match_int(args, &pipefd))
 				return 1;
-			sbi->pipefd = pipefd;
+			ret = autofs_parse_fd(sbi, pipefd);
+			if (ret)
+				return 1;
 			break;
 		case Opt_uid:
 			if (match_int(args, &option))
@@ -222,7 +252,6 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 {
 	struct inode *root_inode;
 	struct dentry *root;
-	struct file *pipe;
 	struct autofs_sb_info *sbi;
 	struct autofs_info *ino;
 	int pgrp = 0;
@@ -275,7 +304,6 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 		ret = -ENOMEM;
 		goto fail_ino;
 	}
-	pipe = NULL;
 
 	root->d_fsdata = ino;
 
@@ -321,16 +349,7 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 
 	pr_debug("pipe fd = %d, pgrp = %u\n",
 		 sbi->pipefd, pid_nr(sbi->oz_pgrp));
-	pipe = fget(sbi->pipefd);
 
-	if (!pipe) {
-		pr_err("could not open pipe file descriptor\n");
-		goto fail_put_pid;
-	}
-	ret = autofs_prepare_pipe(pipe);
-	if (ret < 0)
-		goto fail_fput;
-	sbi->pipe = pipe;
 	sbi->flags &= ~AUTOFS_SBI_CATATONIC;
 
 	/*
@@ -342,11 +361,6 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	/*
 	 * Failure ... clean up.
 	 */
-fail_fput:
-	pr_err("pipe file descriptor does not contain proper ops\n");
-	fput(pipe);
-fail_put_pid:
-	put_pid(sbi->oz_pgrp);
 fail_dput:
 	dput(root);
 	goto fail_free;
-- 
2.41.0

[PATCH 3/8] autofs: refactor super block info init

[Ian Kent]

Move the allocation and initialisation of the super block
info struct to its own function.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 53 +++++++++++++++++++++++++----------------------
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index e279e275b0a5..992d6cb29707 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -171,11 +171,6 @@ static int parse_options(char *options,
 	root->i_uid = current_uid();
 	root->i_gid = current_gid();
 
-	sbi->min_proto = AUTOFS_MIN_PROTO_VERSION;
-	sbi->max_proto = AUTOFS_MAX_PROTO_VERSION;
-
-	sbi->pipefd = -1;
-
 	if (!options)
 		return 1;
 
@@ -248,41 +243,49 @@ static int parse_options(char *options,
 	return (sbi->pipefd < 0);
 }
 
-int autofs_fill_super(struct super_block *s, void *data, int silent)
+static struct autofs_sb_info *autofs_alloc_sbi(void)
 {
-	struct inode *root_inode;
-	struct dentry *root;
 	struct autofs_sb_info *sbi;
-	struct autofs_info *ino;
-	int pgrp = 0;
-	bool pgrp_set = false;
-	int ret = -EINVAL;
 
 	sbi = kzalloc(sizeof(*sbi), GFP_KERNEL);
 	if (!sbi)
-		return -ENOMEM;
-	pr_debug("starting up, sbi = %p\n", sbi);
+		return NULL;
 
-	s->s_fs_info = sbi;
 	sbi->magic = AUTOFS_SBI_MAGIC;
-	sbi->pipefd = -1;
-	sbi->pipe = NULL;
-	sbi->exp_timeout = 0;
-	sbi->oz_pgrp = NULL;
-	sbi->sb = s;
-	sbi->version = 0;
-	sbi->sub_version = 0;
 	sbi->flags = AUTOFS_SBI_CATATONIC;
+	sbi->min_proto = AUTOFS_MIN_PROTO_VERSION;
+	sbi->max_proto = AUTOFS_MAX_PROTO_VERSION;
+	sbi->pipefd = -1;
+
 	set_autofs_type_indirect(&sbi->type);
-	sbi->min_proto = 0;
-	sbi->max_proto = 0;
 	mutex_init(&sbi->wq_mutex);
 	mutex_init(&sbi->pipe_mutex);
 	spin_lock_init(&sbi->fs_lock);
-	sbi->queues = NULL;
 	spin_lock_init(&sbi->lookup_lock);
 	INIT_LIST_HEAD(&sbi->active_list);
 	INIT_LIST_HEAD(&sbi->expiring_list);
+
+	return sbi;
+}
+
+int autofs_fill_super(struct super_block *s, void *data, int silent)
+{
+	struct inode *root_inode;
+	struct dentry *root;
+	struct autofs_sb_info *sbi;
+	struct autofs_info *ino;
+	int pgrp = 0;
+	bool pgrp_set = false;
+	int ret = -EINVAL;
+
+	sbi = autofs_alloc_sbi();
+	if (!sbi)
+		return -ENOMEM;
+
+	pr_debug("starting up, sbi = %p\n", sbi);
+
+	sbi->sb = s;
+	s->s_fs_info = sbi;
 	s->s_blocksize = 1024;
 	s->s_blocksize_bits = 10;
 	s->s_magic = AUTOFS_SUPER_MAGIC;
-- 
2.41.0

[PATCH 4/8] autofs: reformat 0pt enum declaration

[Ian Kent]

The enum of options is only reformated in the patch to convert autofs
to use the mount API so do that now to simplify the conversion patch.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 992d6cb29707..d2b333c0682a 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -110,9 +110,20 @@ static const struct super_operations autofs_sops = {
 	.evict_inode	= autofs_evict_inode,
 };
 
-enum {Opt_err, Opt_fd, Opt_uid, Opt_gid, Opt_pgrp, Opt_minproto, Opt_maxproto,
-	Opt_indirect, Opt_direct, Opt_offset, Opt_strictexpire,
-	Opt_ignore};
+enum {
+	Opt_err,
+	Opt_direct,
+	Opt_fd,
+	Opt_gid,
+	Opt_ignore,
+	Opt_indirect,
+	Opt_maxproto,
+	Opt_minproto,
+	Opt_offset,
+	Opt_pgrp,
+	Opt_strictexpire,
+	Opt_uid,
+};
 
 static const match_table_t tokens = {
 	{Opt_fd, "fd=%u"},
-- 
2.41.0

[PATCH 5/8] autofs: refactor parse_options()

[Ian Kent]

Seperate out parts of parse_options() that will match better the
individual option processing used in the mount API to further simplify
the upcoming conversion.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 136 ++++++++++++++++++++++++----------------------
 1 file changed, 72 insertions(+), 64 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index d2b333c0682a..5e061ce3ab8d 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -167,18 +167,84 @@ static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
 	return 0;
 }
 
-static int parse_options(char *options,
-			 struct inode *root, int *pgrp, bool *pgrp_set,
-			 struct autofs_sb_info *sbi)
+static int autofs_parse_param(char *optstr, struct inode *root,
+			      int *pgrp, bool *pgrp_set,
+			      struct autofs_sb_info *sbi)
 {
-	char *p;
 	substring_t args[MAX_OPT_ARGS];
 	int option;
 	int pipefd = -1;
 	kuid_t uid;
 	kgid_t gid;
+	int token;
 	int ret;
 
+	token = match_token(optstr, tokens, args);
+	switch (token) {
+	case Opt_fd:
+		if (match_int(args, &pipefd))
+			return 1;
+		ret = autofs_parse_fd(sbi, pipefd);
+		if (ret)
+			return 1;
+		break;
+	case Opt_uid:
+		if (match_int(args, &option))
+			return 1;
+		uid = make_kuid(current_user_ns(), option);
+		if (!uid_valid(uid))
+			return 1;
+		root->i_uid = uid;
+		break;
+	case Opt_gid:
+		if (match_int(args, &option))
+			return 1;
+		gid = make_kgid(current_user_ns(), option);
+		if (!gid_valid(gid))
+			return 1;
+		root->i_gid = gid;
+		break;
+	case Opt_pgrp:
+		if (match_int(args, &option))
+			return 1;
+		*pgrp = option;
+		*pgrp_set = true;
+		break;
+	case Opt_minproto:
+		if (match_int(args, &option))
+			return 1;
+		sbi->min_proto = option;
+		break;
+	case Opt_maxproto:
+		if (match_int(args, &option))
+			return 1;
+		sbi->max_proto = option;
+		break;
+	case Opt_indirect:
+		set_autofs_type_indirect(&sbi->type);
+		break;
+	case Opt_direct:
+		set_autofs_type_direct(&sbi->type);
+		break;
+	case Opt_offset:
+		set_autofs_type_offset(&sbi->type);
+		break;
+	case Opt_strictexpire:
+		sbi->flags |= AUTOFS_SBI_STRICTEXPIRE;
+		break;
+	case Opt_ignore:
+		sbi->flags |= AUTOFS_SBI_IGNORE;
+	}
+
+	return 0;
+}
+
+static int parse_options(char *options,
+			 struct inode *root, int *pgrp, bool *pgrp_set,
+			 struct autofs_sb_info *sbi)
+{
+	char *p;
+
 	root->i_uid = current_uid();
 	root->i_gid = current_gid();
 
@@ -186,71 +252,13 @@ static int parse_options(char *options,
 		return 1;
 
 	while ((p = strsep(&options, ",")) != NULL) {
-		int token;
-
 		if (!*p)
 			continue;
 
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_fd:
-			if (match_int(args, &pipefd))
-				return 1;
-			ret = autofs_parse_fd(sbi, pipefd);
-			if (ret)
-				return 1;
-			break;
-		case Opt_uid:
-			if (match_int(args, &option))
-				return 1;
-			uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(uid))
-				return 1;
-			root->i_uid = uid;
-			break;
-		case Opt_gid:
-			if (match_int(args, &option))
-				return 1;
-			gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(gid))
-				return 1;
-			root->i_gid = gid;
-			break;
-		case Opt_pgrp:
-			if (match_int(args, &option))
-				return 1;
-			*pgrp = option;
-			*pgrp_set = true;
-			break;
-		case Opt_minproto:
-			if (match_int(args, &option))
-				return 1;
-			sbi->min_proto = option;
-			break;
-		case Opt_maxproto:
-			if (match_int(args, &option))
-				return 1;
-			sbi->max_proto = option;
-			break;
-		case Opt_indirect:
-			set_autofs_type_indirect(&sbi->type);
-			break;
-		case Opt_direct:
-			set_autofs_type_direct(&sbi->type);
-			break;
-		case Opt_offset:
-			set_autofs_type_offset(&sbi->type);
-			break;
-		case Opt_strictexpire:
-			sbi->flags |= AUTOFS_SBI_STRICTEXPIRE;
-			break;
-		case Opt_ignore:
-			sbi->flags |= AUTOFS_SBI_IGNORE;
-			break;
-		default:
+		if (autofs_parse_param(p, root, pgrp, pgrp_set, sbi))
 			return 1;
-		}
 	}
+
 	return (sbi->pipefd < 0);
 }
 
-- 
2.41.0

[PATCH 6/8] autofs: validate protocol version

[Ian Kent]

Move the protocol parameter validation into a seperate function.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 5e061ce3ab8d..e2026e063d8c 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -287,6 +287,28 @@ static struct autofs_sb_info *autofs_alloc_sbi(void)
 	return sbi;
 }
 
+static int autofs_validate_protocol(struct autofs_sb_info *sbi)
+{
+	/* Test versions first */
+	if (sbi->max_proto < AUTOFS_MIN_PROTO_VERSION ||
+	    sbi->min_proto > AUTOFS_MAX_PROTO_VERSION) {
+		pr_err("kernel does not match daemon version "
+		       "daemon (%d, %d) kernel (%d, %d)\n",
+		       sbi->min_proto, sbi->max_proto,
+		       AUTOFS_MIN_PROTO_VERSION, AUTOFS_MAX_PROTO_VERSION);
+		return -EINVAL;
+	}
+
+	/* Establish highest kernel protocol version */
+	if (sbi->max_proto > AUTOFS_MAX_PROTO_VERSION)
+		sbi->version = AUTOFS_MAX_PROTO_VERSION;
+	else
+		sbi->version = sbi->max_proto;
+	sbi->sub_version = AUTOFS_PROTO_SUBVERSION;
+
+	return 0;
+}
+
 int autofs_fill_super(struct super_block *s, void *data, int silent)
 {
 	struct inode *root_inode;
@@ -335,22 +357,8 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 		goto fail_dput;
 	}
 
-	/* Test versions first */
-	if (sbi->max_proto < AUTOFS_MIN_PROTO_VERSION ||
-	    sbi->min_proto > AUTOFS_MAX_PROTO_VERSION) {
-		pr_err("kernel does not match daemon version "
-		       "daemon (%d, %d) kernel (%d, %d)\n",
-		       sbi->min_proto, sbi->max_proto,
-		       AUTOFS_MIN_PROTO_VERSION, AUTOFS_MAX_PROTO_VERSION);
+	if (autofs_validate_protocol(sbi))
 		goto fail_dput;
-	}
-
-	/* Establish highest kernel protocol version */
-	if (sbi->max_proto > AUTOFS_MAX_PROTO_VERSION)
-		sbi->version = AUTOFS_MAX_PROTO_VERSION;
-	else
-		sbi->version = sbi->max_proto;
-	sbi->sub_version = AUTOFS_PROTO_SUBVERSION;
 
 	if (pgrp_set) {
 		sbi->oz_pgrp = find_get_pid(pgrp);
-- 
2.41.0

[PATCH 7/8] autofs: convert autofs to use the new mount api

[Ian Kent]

Convert the autofs filesystem to use the mount API.

The conversion patch was originally written by David Howells.
I have taken that patch and broken it into several patches in an effort
to make the change easier to review.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/autofs_i.h |   5 +-
 fs/autofs/init.c     |   9 +-
 fs/autofs/inode.c    | 247 ++++++++++++++++++++++++-------------------
 3 files changed, 142 insertions(+), 119 deletions(-)

diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index c24d32be7937..244f18cdf23c 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -25,6 +25,8 @@
 #include <linux/completion.h>
 #include <linux/file.h>
 #include <linux/magic.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 
 /* This is the range of ioctl() numbers we claim as ours */
 #define AUTOFS_IOC_FIRST     AUTOFS_IOC_READY
@@ -205,7 +207,8 @@ static inline void managed_dentry_clear_managed(struct dentry *dentry)
 
 /* Initializing function */
 
-int autofs_fill_super(struct super_block *, void *, int);
+extern const struct fs_parameter_spec autofs_param_specs[];
+int autofs_init_fs_context(struct fs_context *fc);
 struct autofs_info *autofs_new_ino(struct autofs_sb_info *);
 void autofs_clean_ino(struct autofs_info *);
 
diff --git a/fs/autofs/init.c b/fs/autofs/init.c
index d3f55e874338..b5e4dfa04ed0 100644
--- a/fs/autofs/init.c
+++ b/fs/autofs/init.c
@@ -7,16 +7,11 @@
 #include <linux/init.h>
 #include "autofs_i.h"
 
-static struct dentry *autofs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
-{
-	return mount_nodev(fs_type, flags, data, autofs_fill_super);
-}
-
 struct file_system_type autofs_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "autofs",
-	.mount		= autofs_mount,
+	.init_fs_context = autofs_init_fs_context,
+	.parameters	= autofs_param_specs,
 	.kill_sb	= autofs_kill_sb,
 };
 MODULE_ALIAS_FS("autofs");
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index e2026e063d8c..3f2dfed428f9 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -6,7 +6,6 @@
 
 #include <linux/seq_file.h>
 #include <linux/pagemap.h>
-#include <linux/parser.h>
 
 #include "autofs_i.h"
 
@@ -111,7 +110,6 @@ static const struct super_operations autofs_sops = {
 };
 
 enum {
-	Opt_err,
 	Opt_direct,
 	Opt_fd,
 	Opt_gid,
@@ -125,35 +123,48 @@ enum {
 	Opt_uid,
 };
 
-static const match_table_t tokens = {
-	{Opt_fd, "fd=%u"},
-	{Opt_uid, "uid=%u"},
-	{Opt_gid, "gid=%u"},
-	{Opt_pgrp, "pgrp=%u"},
-	{Opt_minproto, "minproto=%u"},
-	{Opt_maxproto, "maxproto=%u"},
-	{Opt_indirect, "indirect"},
-	{Opt_direct, "direct"},
-	{Opt_offset, "offset"},
-	{Opt_strictexpire, "strictexpire"},
-	{Opt_ignore, "ignore"},
-	{Opt_err, NULL}
+const struct fs_parameter_spec autofs_param_specs[] = {
+	fsparam_flag	("direct",		Opt_direct),
+	fsparam_fd	("fd",			Opt_fd),
+	fsparam_u32	("gid",			Opt_gid),
+	fsparam_flag	("ignore",		Opt_ignore),
+	fsparam_flag	("indirect",		Opt_indirect),
+	fsparam_u32	("maxproto",		Opt_maxproto),
+	fsparam_u32	("minproto",		Opt_minproto),
+	fsparam_flag	("offset",		Opt_offset),
+	fsparam_u32	("pgrp",		Opt_pgrp),
+	fsparam_flag	("strictexpire",	Opt_strictexpire),
+	fsparam_u32	("uid",			Opt_uid),
+	{}
 };
 
-static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
+struct autofs_fs_context {
+	kuid_t	uid;
+	kgid_t	gid;
+	int	pgrp;
+	bool	pgrp_set;
+};
+
+/*
+ * Open the fd.  We do it here rather than in get_tree so that it's done in the
+ * context of the system call that passed the data and not the one that
+ * triggered the superblock creation, lest the fd gets reassigned.
+ */
+static int autofs_parse_fd(struct fs_context *fc, int fd)
 {
+	struct autofs_sb_info *sbi = fc->s_fs_info;
 	struct file *pipe;
 	int ret;
 
 	pipe = fget(fd);
 	if (!pipe) {
-		pr_err("could not open pipe file descriptor\n");
+		errorf(fc, "could not open pipe file descriptor");
 		return -EBADF;
 	}
 
 	ret = autofs_check_pipe(pipe);
 	if (ret < 0) {
-		pr_err("Invalid/unusable pipe\n");
+		errorf(fc, "Invalid/unusable pipe");
 		fput(pipe);
 		return -EBADF;
 	}
@@ -167,58 +178,43 @@ static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
 	return 0;
 }
 
-static int autofs_parse_param(char *optstr, struct inode *root,
-			      int *pgrp, bool *pgrp_set,
-			      struct autofs_sb_info *sbi)
+static int autofs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	substring_t args[MAX_OPT_ARGS];
-	int option;
-	int pipefd = -1;
+	struct autofs_fs_context *ctx = fc->fs_private;
+	struct autofs_sb_info *sbi = fc->s_fs_info;
+	struct fs_parse_result result;
 	kuid_t uid;
 	kgid_t gid;
-	int token;
-	int ret;
+	int opt;
 
-	token = match_token(optstr, tokens, args);
-	switch (token) {
+	opt = fs_parse(fc, autofs_param_specs, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
 	case Opt_fd:
-		if (match_int(args, &pipefd))
-			return 1;
-		ret = autofs_parse_fd(sbi, pipefd);
-		if (ret)
-			return 1;
-		break;
+		return autofs_parse_fd(fc, result.int_32);
 	case Opt_uid:
-		if (match_int(args, &option))
-			return 1;
-		uid = make_kuid(current_user_ns(), option);
+		uid = make_kuid(current_user_ns(), result.uint_32);
 		if (!uid_valid(uid))
 			return 1;
-		root->i_uid = uid;
+		ctx->uid = uid;
 		break;
 	case Opt_gid:
-		if (match_int(args, &option))
-			return 1;
-		gid = make_kgid(current_user_ns(), option);
+		gid = make_kgid(current_user_ns(), result.uint_32);
 		if (!gid_valid(gid))
 			return 1;
-		root->i_gid = gid;
+		ctx->gid = gid;
 		break;
 	case Opt_pgrp:
-		if (match_int(args, &option))
-			return 1;
-		*pgrp = option;
-		*pgrp_set = true;
+		ctx->pgrp = result.uint_32;
+		ctx->pgrp_set = true;
 		break;
 	case Opt_minproto:
-		if (match_int(args, &option))
-			return 1;
-		sbi->min_proto = option;
+		sbi->min_proto = result.uint_32;
 		break;
 	case Opt_maxproto:
-		if (match_int(args, &option))
-			return 1;
-		sbi->max_proto = option;
+		sbi->max_proto = result.uint_32;
 		break;
 	case Opt_indirect:
 		set_autofs_type_indirect(&sbi->type);
@@ -239,29 +235,6 @@ static int autofs_parse_param(char *optstr, struct inode *root,
 	return 0;
 }
 
-static int parse_options(char *options,
-			 struct inode *root, int *pgrp, bool *pgrp_set,
-			 struct autofs_sb_info *sbi)
-{
-	char *p;
-
-	root->i_uid = current_uid();
-	root->i_gid = current_gid();
-
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		if (autofs_parse_param(p, root, pgrp, pgrp_set, sbi))
-			return 1;
-	}
-
-	return (sbi->pipefd < 0);
-}
-
 static struct autofs_sb_info *autofs_alloc_sbi(void)
 {
 	struct autofs_sb_info *sbi;
@@ -287,12 +260,14 @@ static struct autofs_sb_info *autofs_alloc_sbi(void)
 	return sbi;
 }
 
-static int autofs_validate_protocol(struct autofs_sb_info *sbi)
+static int autofs_validate_protocol(struct fs_context *fc)
 {
+	struct autofs_sb_info *sbi = fc->s_fs_info;
+
 	/* Test versions first */
 	if (sbi->max_proto < AUTOFS_MIN_PROTO_VERSION ||
 	    sbi->min_proto > AUTOFS_MAX_PROTO_VERSION) {
-		pr_err("kernel does not match daemon version "
+		errorf(fc, "kernel does not match daemon version "
 		       "daemon (%d, %d) kernel (%d, %d)\n",
 		       sbi->min_proto, sbi->max_proto,
 		       AUTOFS_MIN_PROTO_VERSION, AUTOFS_MAX_PROTO_VERSION);
@@ -309,24 +284,18 @@ static int autofs_validate_protocol(struct autofs_sb_info *sbi)
 	return 0;
 }
 
-int autofs_fill_super(struct super_block *s, void *data, int silent)
+static int autofs_fill_super(struct super_block *s, struct fs_context *fc)
 {
+	struct autofs_fs_context *ctx = fc->fs_private;
+	struct autofs_sb_info *sbi = s->s_fs_info;
 	struct inode *root_inode;
 	struct dentry *root;
-	struct autofs_sb_info *sbi;
 	struct autofs_info *ino;
-	int pgrp = 0;
-	bool pgrp_set = false;
-	int ret = -EINVAL;
-
-	sbi = autofs_alloc_sbi();
-	if (!sbi)
-		return -ENOMEM;
+	int ret = -ENOMEM;
 
 	pr_debug("starting up, sbi = %p\n", sbi);
 
 	sbi->sb = s;
-	s->s_fs_info = sbi;
 	s->s_blocksize = 1024;
 	s->s_blocksize_bits = 10;
 	s->s_magic = AUTOFS_SUPER_MAGIC;
@@ -338,33 +307,24 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	 * Get the root inode and dentry, but defer checking for errors.
 	 */
 	ino = autofs_new_ino(sbi);
-	if (!ino) {
-		ret = -ENOMEM;
-		goto fail_free;
-	}
+	if (!ino)
+		goto fail;
+
 	root_inode = autofs_get_inode(s, S_IFDIR | 0755);
+	root_inode->i_uid = ctx->uid;
+	root_inode->i_gid = ctx->gid;
+
 	root = d_make_root(root_inode);
-	if (!root) {
-		ret = -ENOMEM;
+	if (!root)
 		goto fail_ino;
-	}
 
 	root->d_fsdata = ino;
 
-	/* Can this call block? */
-	if (parse_options(data, root_inode, &pgrp, &pgrp_set, sbi)) {
-		pr_err("called with bogus options\n");
-		goto fail_dput;
-	}
-
-	if (autofs_validate_protocol(sbi))
-		goto fail_dput;
-
-	if (pgrp_set) {
-		sbi->oz_pgrp = find_get_pid(pgrp);
+	if (ctx->pgrp_set) {
+		sbi->oz_pgrp = find_get_pid(ctx->pgrp);
 		if (!sbi->oz_pgrp) {
-			pr_err("could not find process group %d\n",
-				pgrp);
+			ret = invalf(fc, "Could not find process group %d",
+				     ctx->pgrp);
 			goto fail_dput;
 		}
 	} else {
@@ -393,15 +353,80 @@ int autofs_fill_super(struct super_block *s, void *data, int silent)
 	 */
 fail_dput:
 	dput(root);
-	goto fail_free;
+	goto fail;
 fail_ino:
 	autofs_free_ino(ino);
-fail_free:
-	kfree(sbi);
-	s->s_fs_info = NULL;
+fail:
 	return ret;
 }
 
+/*
+ * Validate the parameters and then request a superblock.
+ */
+static int autofs_get_tree(struct fs_context *fc)
+{
+	struct autofs_sb_info *sbi = fc->s_fs_info;
+	int ret;
+
+	ret = autofs_validate_protocol(fc);
+	if (ret)
+		return ret;
+
+	if (sbi->pipefd < 0)
+		return invalf(fc, "No control pipe specified");
+
+	return get_tree_nodev(fc, autofs_fill_super);
+}
+
+static void autofs_free_fc(struct fs_context *fc)
+{
+	struct autofs_fs_context *ctx = fc->fs_private;
+	struct autofs_sb_info *sbi = fc->s_fs_info;
+
+	if (sbi) {
+		if (sbi->pipe)
+			fput(sbi->pipe);
+		kfree(sbi);
+	}
+	kfree(ctx);
+}
+
+static const struct fs_context_operations autofs_context_ops = {
+	.free		= autofs_free_fc,
+	.parse_param	= autofs_parse_param,
+	.get_tree	= autofs_get_tree,
+};
+
+/*
+ * Set up the filesystem mount context.
+ */
+int autofs_init_fs_context(struct fs_context *fc)
+{
+	struct autofs_fs_context *ctx;
+	struct autofs_sb_info *sbi;
+
+	ctx = kzalloc(sizeof(struct autofs_fs_context), GFP_KERNEL);
+	if (!ctx)
+		goto nomem;
+
+	ctx->uid = current_uid();
+	ctx->gid = current_gid();
+
+	sbi = autofs_alloc_sbi();
+	if (!sbi)
+		goto nomem_ctx;
+
+	fc->fs_private = ctx;
+	fc->s_fs_info = sbi;
+	fc->ops = &autofs_context_ops;
+	return 0;
+
+nomem_ctx:
+	kfree(ctx);
+nomem:
+	return -ENOMEM;
+}
+
 struct inode *autofs_get_inode(struct super_block *sb, umode_t mode)
 {
 	struct inode *inode = new_inode(sb);
-- 
2.41.0

Re: [PATCH 7/8] autofs: convert autofs to use the new mount api

[Christian Brauner]

On Fri, Sep 22, 2023 at 12:12:14PM +0800, Ian Kent wrote:
> Convert the autofs filesystem to use the mount API.
> 
> The conversion patch was originally written by David Howells.
> I have taken that patch and broken it into several patches in an effort
> to make the change easier to review.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> ---
>  fs/autofs/autofs_i.h |   5 +-
>  fs/autofs/init.c     |   9 +-
>  fs/autofs/inode.c    | 247 ++++++++++++++++++++++++-------------------
>  3 files changed, 142 insertions(+), 119 deletions(-)
> 
> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
> index c24d32be7937..244f18cdf23c 100644
> --- a/fs/autofs/autofs_i.h
> +++ b/fs/autofs/autofs_i.h
> @@ -25,6 +25,8 @@
>  #include <linux/completion.h>
>  #include <linux/file.h>
>  #include <linux/magic.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  
>  /* This is the range of ioctl() numbers we claim as ours */
>  #define AUTOFS_IOC_FIRST     AUTOFS_IOC_READY
> @@ -205,7 +207,8 @@ static inline void managed_dentry_clear_managed(struct dentry *dentry)
>  
>  /* Initializing function */
>  
> -int autofs_fill_super(struct super_block *, void *, int);
> +extern const struct fs_parameter_spec autofs_param_specs[];
> +int autofs_init_fs_context(struct fs_context *fc);
>  struct autofs_info *autofs_new_ino(struct autofs_sb_info *);
>  void autofs_clean_ino(struct autofs_info *);
>  
> diff --git a/fs/autofs/init.c b/fs/autofs/init.c
> index d3f55e874338..b5e4dfa04ed0 100644
> --- a/fs/autofs/init.c
> +++ b/fs/autofs/init.c
> @@ -7,16 +7,11 @@
>  #include <linux/init.h>
>  #include "autofs_i.h"
>  
> -static struct dentry *autofs_mount(struct file_system_type *fs_type,
> -	int flags, const char *dev_name, void *data)
> -{
> -	return mount_nodev(fs_type, flags, data, autofs_fill_super);
> -}
> -
>  struct file_system_type autofs_fs_type = {
>  	.owner		= THIS_MODULE,
>  	.name		= "autofs",
> -	.mount		= autofs_mount,
> +	.init_fs_context = autofs_init_fs_context,
> +	.parameters	= autofs_param_specs,
>  	.kill_sb	= autofs_kill_sb,
>  };
>  MODULE_ALIAS_FS("autofs");
> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
> index e2026e063d8c..3f2dfed428f9 100644
> --- a/fs/autofs/inode.c
> +++ b/fs/autofs/inode.c
> @@ -6,7 +6,6 @@
>  
>  #include <linux/seq_file.h>
>  #include <linux/pagemap.h>
> -#include <linux/parser.h>
>  
>  #include "autofs_i.h"
>  
> @@ -111,7 +110,6 @@ static const struct super_operations autofs_sops = {
>  };
>  
>  enum {
> -	Opt_err,
>  	Opt_direct,
>  	Opt_fd,
>  	Opt_gid,
> @@ -125,35 +123,48 @@ enum {
>  	Opt_uid,
>  };
>  
> -static const match_table_t tokens = {
> -	{Opt_fd, "fd=%u"},
> -	{Opt_uid, "uid=%u"},
> -	{Opt_gid, "gid=%u"},
> -	{Opt_pgrp, "pgrp=%u"},
> -	{Opt_minproto, "minproto=%u"},
> -	{Opt_maxproto, "maxproto=%u"},
> -	{Opt_indirect, "indirect"},
> -	{Opt_direct, "direct"},
> -	{Opt_offset, "offset"},
> -	{Opt_strictexpire, "strictexpire"},
> -	{Opt_ignore, "ignore"},
> -	{Opt_err, NULL}
> +const struct fs_parameter_spec autofs_param_specs[] = {
> +	fsparam_flag	("direct",		Opt_direct),
> +	fsparam_fd	("fd",			Opt_fd),
> +	fsparam_u32	("gid",			Opt_gid),
> +	fsparam_flag	("ignore",		Opt_ignore),
> +	fsparam_flag	("indirect",		Opt_indirect),
> +	fsparam_u32	("maxproto",		Opt_maxproto),
> +	fsparam_u32	("minproto",		Opt_minproto),
> +	fsparam_flag	("offset",		Opt_offset),
> +	fsparam_u32	("pgrp",		Opt_pgrp),
> +	fsparam_flag	("strictexpire",	Opt_strictexpire),
> +	fsparam_u32	("uid",			Opt_uid),
> +	{}
>  };
>  
> -static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
> +struct autofs_fs_context {
> +	kuid_t	uid;
> +	kgid_t	gid;
> +	int	pgrp;
> +	bool	pgrp_set;
> +};
> +
> +/*
> + * Open the fd.  We do it here rather than in get_tree so that it's done in the
> + * context of the system call that passed the data and not the one that
> + * triggered the superblock creation, lest the fd gets reassigned.
> + */
> +static int autofs_parse_fd(struct fs_context *fc, int fd)
>  {
> +	struct autofs_sb_info *sbi = fc->s_fs_info;
>  	struct file *pipe;
>  	int ret;
>  
>  	pipe = fget(fd);
>  	if (!pipe) {
> -		pr_err("could not open pipe file descriptor\n");
> +		errorf(fc, "could not open pipe file descriptor");
>  		return -EBADF;
>  	}
>  
>  	ret = autofs_check_pipe(pipe);
>  	if (ret < 0) {
> -		pr_err("Invalid/unusable pipe\n");
> +		errorf(fc, "Invalid/unusable pipe");
>  		fput(pipe);
>  		return -EBADF;
>  	}
> @@ -167,58 +178,43 @@ static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
>  	return 0;
>  }
>  
> -static int autofs_parse_param(char *optstr, struct inode *root,
> -			      int *pgrp, bool *pgrp_set,
> -			      struct autofs_sb_info *sbi)
> +static int autofs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	substring_t args[MAX_OPT_ARGS];
> -	int option;
> -	int pipefd = -1;
> +	struct autofs_fs_context *ctx = fc->fs_private;
> +	struct autofs_sb_info *sbi = fc->s_fs_info;
> +	struct fs_parse_result result;
>  	kuid_t uid;
>  	kgid_t gid;
> -	int token;
> -	int ret;
> +	int opt;
>  
> -	token = match_token(optstr, tokens, args);
> -	switch (token) {
> +	opt = fs_parse(fc, autofs_param_specs, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
>  	case Opt_fd:
> -		if (match_int(args, &pipefd))
> -			return 1;
> -		ret = autofs_parse_fd(sbi, pipefd);
> -		if (ret)
> -			return 1;
> -		break;
> +		return autofs_parse_fd(fc, result.int_32);
>  	case Opt_uid:
> -		if (match_int(args, &option))
> -			return 1;
> -		uid = make_kuid(current_user_ns(), option);
> +		uid = make_kuid(current_user_ns(), result.uint_32);
>  		if (!uid_valid(uid))
>  			return 1;

This and the make_kgid() instance below need to return -EINVAL or use
invalfc() to return an error message. I can fix this up though so no
need to resend for this.

Re: [PATCH 7/8] autofs: convert autofs to use the new mount api

[Ian Kent]

On 22/9/23 16:31, Christian Brauner wrote:
> On Fri, Sep 22, 2023 at 12:12:14PM +0800, Ian Kent wrote:
>> Convert the autofs filesystem to use the mount API.
>>
>> The conversion patch was originally written by David Howells.
>> I have taken that patch and broken it into several patches in an effort
>> to make the change easier to review.
>>
>> Signed-off-by: Ian Kent <raven@themaw.net>
>> ---
>>   fs/autofs/autofs_i.h |   5 +-
>>   fs/autofs/init.c     |   9 +-
>>   fs/autofs/inode.c    | 247 ++++++++++++++++++++++++-------------------
>>   3 files changed, 142 insertions(+), 119 deletions(-)
>>
>> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
>> index c24d32be7937..244f18cdf23c 100644
>> --- a/fs/autofs/autofs_i.h
>> +++ b/fs/autofs/autofs_i.h
>> @@ -25,6 +25,8 @@
>>   #include <linux/completion.h>
>>   #include <linux/file.h>
>>   #include <linux/magic.h>
>> +#include <linux/fs_context.h>
>> +#include <linux/fs_parser.h>
>>   
>>   /* This is the range of ioctl() numbers we claim as ours */
>>   #define AUTOFS_IOC_FIRST     AUTOFS_IOC_READY
>> @@ -205,7 +207,8 @@ static inline void managed_dentry_clear_managed(struct dentry *dentry)
>>   
>>   /* Initializing function */
>>   
>> -int autofs_fill_super(struct super_block *, void *, int);
>> +extern const struct fs_parameter_spec autofs_param_specs[];
>> +int autofs_init_fs_context(struct fs_context *fc);
>>   struct autofs_info *autofs_new_ino(struct autofs_sb_info *);
>>   void autofs_clean_ino(struct autofs_info *);
>>   
>> diff --git a/fs/autofs/init.c b/fs/autofs/init.c
>> index d3f55e874338..b5e4dfa04ed0 100644
>> --- a/fs/autofs/init.c
>> +++ b/fs/autofs/init.c
>> @@ -7,16 +7,11 @@
>>   #include <linux/init.h>
>>   #include "autofs_i.h"
>>   
>> -static struct dentry *autofs_mount(struct file_system_type *fs_type,
>> -	int flags, const char *dev_name, void *data)
>> -{
>> -	return mount_nodev(fs_type, flags, data, autofs_fill_super);
>> -}
>> -
>>   struct file_system_type autofs_fs_type = {
>>   	.owner		= THIS_MODULE,
>>   	.name		= "autofs",
>> -	.mount		= autofs_mount,
>> +	.init_fs_context = autofs_init_fs_context,
>> +	.parameters	= autofs_param_specs,
>>   	.kill_sb	= autofs_kill_sb,
>>   };
>>   MODULE_ALIAS_FS("autofs");
>> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
>> index e2026e063d8c..3f2dfed428f9 100644
>> --- a/fs/autofs/inode.c
>> +++ b/fs/autofs/inode.c
>> @@ -6,7 +6,6 @@
>>   
>>   #include <linux/seq_file.h>
>>   #include <linux/pagemap.h>
>> -#include <linux/parser.h>
>>   
>>   #include "autofs_i.h"
>>   
>> @@ -111,7 +110,6 @@ static const struct super_operations autofs_sops = {
>>   };
>>   
>>   enum {
>> -	Opt_err,
>>   	Opt_direct,
>>   	Opt_fd,
>>   	Opt_gid,
>> @@ -125,35 +123,48 @@ enum {
>>   	Opt_uid,
>>   };
>>   
>> -static const match_table_t tokens = {
>> -	{Opt_fd, "fd=%u"},
>> -	{Opt_uid, "uid=%u"},
>> -	{Opt_gid, "gid=%u"},
>> -	{Opt_pgrp, "pgrp=%u"},
>> -	{Opt_minproto, "minproto=%u"},
>> -	{Opt_maxproto, "maxproto=%u"},
>> -	{Opt_indirect, "indirect"},
>> -	{Opt_direct, "direct"},
>> -	{Opt_offset, "offset"},
>> -	{Opt_strictexpire, "strictexpire"},
>> -	{Opt_ignore, "ignore"},
>> -	{Opt_err, NULL}
>> +const struct fs_parameter_spec autofs_param_specs[] = {
>> +	fsparam_flag	("direct",		Opt_direct),
>> +	fsparam_fd	("fd",			Opt_fd),
>> +	fsparam_u32	("gid",			Opt_gid),
>> +	fsparam_flag	("ignore",		Opt_ignore),
>> +	fsparam_flag	("indirect",		Opt_indirect),
>> +	fsparam_u32	("maxproto",		Opt_maxproto),
>> +	fsparam_u32	("minproto",		Opt_minproto),
>> +	fsparam_flag	("offset",		Opt_offset),
>> +	fsparam_u32	("pgrp",		Opt_pgrp),
>> +	fsparam_flag	("strictexpire",	Opt_strictexpire),
>> +	fsparam_u32	("uid",			Opt_uid),
>> +	{}
>>   };
>>   
>> -static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
>> +struct autofs_fs_context {
>> +	kuid_t	uid;
>> +	kgid_t	gid;
>> +	int	pgrp;
>> +	bool	pgrp_set;
>> +};
>> +
>> +/*
>> + * Open the fd.  We do it here rather than in get_tree so that it's done in the
>> + * context of the system call that passed the data and not the one that
>> + * triggered the superblock creation, lest the fd gets reassigned.
>> + */
>> +static int autofs_parse_fd(struct fs_context *fc, int fd)
>>   {
>> +	struct autofs_sb_info *sbi = fc->s_fs_info;
>>   	struct file *pipe;
>>   	int ret;
>>   
>>   	pipe = fget(fd);
>>   	if (!pipe) {
>> -		pr_err("could not open pipe file descriptor\n");
>> +		errorf(fc, "could not open pipe file descriptor");
>>   		return -EBADF;
>>   	}
>>   
>>   	ret = autofs_check_pipe(pipe);
>>   	if (ret < 0) {
>> -		pr_err("Invalid/unusable pipe\n");
>> +		errorf(fc, "Invalid/unusable pipe");
>>   		fput(pipe);
>>   		return -EBADF;
>>   	}
>> @@ -167,58 +178,43 @@ static int autofs_parse_fd(struct autofs_sb_info *sbi, int fd)
>>   	return 0;
>>   }
>>   
>> -static int autofs_parse_param(char *optstr, struct inode *root,
>> -			      int *pgrp, bool *pgrp_set,
>> -			      struct autofs_sb_info *sbi)
>> +static int autofs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>>   {
>> -	substring_t args[MAX_OPT_ARGS];
>> -	int option;
>> -	int pipefd = -1;
>> +	struct autofs_fs_context *ctx = fc->fs_private;
>> +	struct autofs_sb_info *sbi = fc->s_fs_info;
>> +	struct fs_parse_result result;
>>   	kuid_t uid;
>>   	kgid_t gid;
>> -	int token;
>> -	int ret;
>> +	int opt;
>>   
>> -	token = match_token(optstr, tokens, args);
>> -	switch (token) {
>> +	opt = fs_parse(fc, autofs_param_specs, param, &result);
>> +	if (opt < 0)
>> +		return opt;
>> +
>> +	switch (opt) {
>>   	case Opt_fd:
>> -		if (match_int(args, &pipefd))
>> -			return 1;
>> -		ret = autofs_parse_fd(sbi, pipefd);
>> -		if (ret)
>> -			return 1;
>> -		break;
>> +		return autofs_parse_fd(fc, result.int_32);
>>   	case Opt_uid:
>> -		if (match_int(args, &option))
>> -			return 1;
>> -		uid = make_kuid(current_user_ns(), option);
>> +		uid = make_kuid(current_user_ns(), result.uint_32);
>>   		if (!uid_valid(uid))
>>   			return 1;
> This and the make_kgid() instance below need to return -EINVAL or use
> invalfc() to return an error message. I can fix this up though so no
> need to resend for this.


Right you are, sorry about that and thanks very much for fixing it for

me.


Ian

Ian

Re: [PATCH 7/8] autofs: convert autofs to use the new mount api

[Christian Brauner]

> +	fsparam_fd	("fd",			Opt_fd),

> +/*
> + * Open the fd.  We do it here rather than in get_tree so that it's done in the
> + * context of the system call that passed the data and not the one that
> + * triggered the superblock creation, lest the fd gets reassigned.
> + */
> +static int autofs_parse_fd(struct fs_context *fc, int fd)
>  {
> +	struct autofs_sb_info *sbi = fc->s_fs_info;
>  	struct file *pipe;
>  	int ret;
>  
>  	pipe = fget(fd);
>  	if (!pipe) {
> -		pr_err("could not open pipe file descriptor\n");
> +		errorf(fc, "could not open pipe file descriptor");
>  		return -EBADF;
>  	}
>  
>  	ret = autofs_check_pipe(pipe);
>  	if (ret < 0) {
> -		pr_err("Invalid/unusable pipe\n");
> +		errorf(fc, "Invalid/unusable pipe");
>  		fput(pipe);
>  		return -EBADF;
>  	}

> +static int autofs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> +		return autofs_parse_fd(fc, result.int_32);

Mah, so there's a difference between the new and the old mount api that we
should probably hide on the VFS level for fsparam_fd. Basically, if you're
coming through the new mount api via fsconfig(FSCONFIG_SET_FD, fd) then the vfs
will have done param->file = fget(fd) for you already so there's no need to
call fget() again. We can just take ownership of the reference that the vfs
took for us.

But if we're coming in through the old mount api then we need to call fget.
There's nothing wrong with your code but it doesn't take advantage of the new
mount api which would be unfortunate. So I folded a small extension into this
see [1].

There's an unrelated bug in fs_param_is_fd() that I'm also fixing see [2].

I've tested both changes with the old and new mount api.

[1]:
diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 3f2dfed428f9..0477bce7d277 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -150,13 +150,20 @@ struct autofs_fs_context {
  * context of the system call that passed the data and not the one that
  * triggered the superblock creation, lest the fd gets reassigned.
  */
-static int autofs_parse_fd(struct fs_context *fc, int fd)
+static int autofs_parse_fd(struct fs_context *fc, struct autofs_sb_info *sbi,
+                          struct fs_parameter *param,
+                          struct fs_parse_result *result)
 {
-       struct autofs_sb_info *sbi = fc->s_fs_info;
        struct file *pipe;
        int ret;

-       pipe = fget(fd);
+       if (param->type == fs_value_is_file) {
+               /* came through the new api */
+               pipe = param->file;
+               param->file = NULL;
+       } else {
+               pipe = fget(result->uint_32);
+       }
        if (!pipe) {
                errorf(fc, "could not open pipe file descriptor");
                return -EBADF;
@@ -165,14 +172,15 @@ static int autofs_parse_fd(struct fs_context *fc, int fd)
        ret = autofs_check_pipe(pipe);
        if (ret < 0) {
                errorf(fc, "Invalid/unusable pipe");
-               fput(pipe);
+               if (param->type != fs_value_is_file)
+                       fput(pipe);
                return -EBADF;
        }

        if (sbi->pipe)
                fput(sbi->pipe);

-       sbi->pipefd = fd;
+       sbi->pipefd = result->uint_32;
        sbi->pipe = pipe;

        return 0;

[2]:
From 2f9171200505c82e744a235c85377e36ed190109 Mon Sep 17 00:00:00 2001
From: Christian Brauner <brauner@kernel.org>
Date: Fri, 22 Sep 2023 13:49:05 +0200
Subject: [PATCH] fsconfig: ensure that dirfd is set to aux

The code in fs_param_is_fd() expects param->dirfd to be set to the fd
that was used to set param->file to initialize result->uint_32. So make
sure it's set so users like autofs using FSCONFIG_SET_FD with the new
mount api can rely on this to be set to the correct value.

Signed-off-by: Christian Brauner <brauner@kernel.org>
---
 fs/fsopen.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fsopen.c b/fs/fsopen.c
index ce03f6521c88..6593ae518115 100644
--- a/fs/fsopen.c
+++ b/fs/fsopen.c
@@ -465,6 +465,7 @@ SYSCALL_DEFINE5(fsconfig,
 		param.file = fget(aux);
 		if (!param.file)
 			goto out_key;
+		param.dirfd = aux;
 		break;
 	default:
 		break;
-- 
2.34.1

Re: [PATCH 7/8] autofs: convert autofs to use the new mount api

[Ian Kent]

On 22/9/23 19:59, Christian Brauner wrote:
>> +	fsparam_fd	("fd",			Opt_fd),
>> +/*
>> + * Open the fd.  We do it here rather than in get_tree so that it's done in the
>> + * context of the system call that passed the data and not the one that
>> + * triggered the superblock creation, lest the fd gets reassigned.
>> + */
>> +static int autofs_parse_fd(struct fs_context *fc, int fd)
>>   {
>> +	struct autofs_sb_info *sbi = fc->s_fs_info;
>>   	struct file *pipe;
>>   	int ret;
>>   
>>   	pipe = fget(fd);
>>   	if (!pipe) {
>> -		pr_err("could not open pipe file descriptor\n");
>> +		errorf(fc, "could not open pipe file descriptor");
>>   		return -EBADF;
>>   	}
>>   
>>   	ret = autofs_check_pipe(pipe);
>>   	if (ret < 0) {
>> -		pr_err("Invalid/unusable pipe\n");
>> +		errorf(fc, "Invalid/unusable pipe");
>>   		fput(pipe);
>>   		return -EBADF;
>>   	}
>> +static int autofs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>>   {
>> +		return autofs_parse_fd(fc, result.int_32);
> Mah, so there's a difference between the new and the old mount api that we
> should probably hide on the VFS level for fsparam_fd. Basically, if you're
> coming through the new mount api via fsconfig(FSCONFIG_SET_FD, fd) then the vfs
> will have done param->file = fget(fd) for you already so there's no need to
> call fget() again. We can just take ownership of the reference that the vfs
> took for us.
>
> But if we're coming in through the old mount api then we need to call fget.
> There's nothing wrong with your code but it doesn't take advantage of the new
> mount api which would be unfortunate. So I folded a small extension into this
> see [1].
>
> There's an unrelated bug in fs_param_is_fd() that I'm also fixing see [2].

Ok, that's interesting, I'll have a look at these developments tomorrow,

admittedly (obviously) I hadn't looked at the code for some time ...


Ian

>
> I've tested both changes with the old and new mount api.
>
> [1]:
> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
> index 3f2dfed428f9..0477bce7d277 100644
> --- a/fs/autofs/inode.c
> +++ b/fs/autofs/inode.c
> @@ -150,13 +150,20 @@ struct autofs_fs_context {
>    * context of the system call that passed the data and not the one that
>    * triggered the superblock creation, lest the fd gets reassigned.
>    */
> -static int autofs_parse_fd(struct fs_context *fc, int fd)
> +static int autofs_parse_fd(struct fs_context *fc, struct autofs_sb_info *sbi,
> +                          struct fs_parameter *param,
> +                          struct fs_parse_result *result)
>   {
> -       struct autofs_sb_info *sbi = fc->s_fs_info;
>          struct file *pipe;
>          int ret;
>
> -       pipe = fget(fd);
> +       if (param->type == fs_value_is_file) {
> +               /* came through the new api */
> +               pipe = param->file;
> +               param->file = NULL;
> +       } else {
> +               pipe = fget(result->uint_32);
> +       }
>          if (!pipe) {
>                  errorf(fc, "could not open pipe file descriptor");
>                  return -EBADF;
> @@ -165,14 +172,15 @@ static int autofs_parse_fd(struct fs_context *fc, int fd)
>          ret = autofs_check_pipe(pipe);
>          if (ret < 0) {
>                  errorf(fc, "Invalid/unusable pipe");
> -               fput(pipe);
> +               if (param->type != fs_value_is_file)
> +                       fput(pipe);
>                  return -EBADF;
>          }
>
>          if (sbi->pipe)
>                  fput(sbi->pipe);
>
> -       sbi->pipefd = fd;
> +       sbi->pipefd = result->uint_32;
>          sbi->pipe = pipe;
>
>          return 0;
>
> [2]:
>  From 2f9171200505c82e744a235c85377e36ed190109 Mon Sep 17 00:00:00 2001
> From: Christian Brauner <brauner@kernel.org>
> Date: Fri, 22 Sep 2023 13:49:05 +0200
> Subject: [PATCH] fsconfig: ensure that dirfd is set to aux
>
> The code in fs_param_is_fd() expects param->dirfd to be set to the fd
> that was used to set param->file to initialize result->uint_32. So make
> sure it's set so users like autofs using FSCONFIG_SET_FD with the new
> mount api can rely on this to be set to the correct value.
>
> Signed-off-by: Christian Brauner <brauner@kernel.org>
> ---
>   fs/fsopen.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/fs/fsopen.c b/fs/fsopen.c
> index ce03f6521c88..6593ae518115 100644
> --- a/fs/fsopen.c
> +++ b/fs/fsopen.c
> @@ -465,6 +465,7 @@ SYSCALL_DEFINE5(fsconfig,
>   		param.file = fget(aux);
>   		if (!param.file)
>   			goto out_key;
> +		param.dirfd = aux;
>   		break;
>   	default:
>   		break;

Re: [PATCH 7/8] autofs: convert autofs to use the new mount api

[Ian Kent]

On 22/9/23 21:27, Ian Kent wrote:
> On 22/9/23 19:59, Christian Brauner wrote:
>>> +    fsparam_fd    ("fd", Opt_fd),
>>> +/*
>>> + * Open the fd.  We do it here rather than in get_tree so that it's 
>>> done in the
>>> + * context of the system call that passed the data and not the one 
>>> that
>>> + * triggered the superblock creation, lest the fd gets reassigned.
>>> + */
>>> +static int autofs_parse_fd(struct fs_context *fc, int fd)
>>>   {
>>> +    struct autofs_sb_info *sbi = fc->s_fs_info;
>>>       struct file *pipe;
>>>       int ret;
>>>         pipe = fget(fd);
>>>       if (!pipe) {
>>> -        pr_err("could not open pipe file descriptor\n");
>>> +        errorf(fc, "could not open pipe file descriptor");
>>>           return -EBADF;
>>>       }
>>>         ret = autofs_check_pipe(pipe);
>>>       if (ret < 0) {
>>> -        pr_err("Invalid/unusable pipe\n");
>>> +        errorf(fc, "Invalid/unusable pipe");
>>>           fput(pipe);
>>>           return -EBADF;
>>>       }
>>> +static int autofs_parse_param(struct fs_context *fc, struct 
>>> fs_parameter *param)
>>>   {
>>> +        return autofs_parse_fd(fc, result.int_32);
>> Mah, so there's a difference between the new and the old mount api 
>> that we
>> should probably hide on the VFS level for fsparam_fd. Basically, if 
>> you're
>> coming through the new mount api via fsconfig(FSCONFIG_SET_FD, fd) 
>> then the vfs
>> will have done param->file = fget(fd) for you already so there's no 
>> need to
>> call fget() again. We can just take ownership of the reference that 
>> the vfs
>> took for us.
>>
>> But if we're coming in through the old mount api then we need to call 
>> fget.
>> There's nothing wrong with your code but it doesn't take advantage of 
>> the new
>> mount api which would be unfortunate. So I folded a small extension 
>> into this
>> see [1].
>>
>> There's an unrelated bug in fs_param_is_fd() that I'm also fixing see 
>> [2].
>
> Ok, that's interesting, I'll have a look at these developments tomorrow,
>
> admittedly (obviously) I hadn't looked at the code for some time ...

This code pattern is a bit unusual, it looks a bit untidy to have different

behavior between the two but I expect there was a reason to include the fd

handling and have this small difference anyway ...


In [2] that's a good catch, I certainly was caught by it, ;(


>
>
> Ian
>
>>
>> I've tested both changes with the old and new mount api.
>>
>> [1]:
>> diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
>> index 3f2dfed428f9..0477bce7d277 100644
>> --- a/fs/autofs/inode.c
>> +++ b/fs/autofs/inode.c
>> @@ -150,13 +150,20 @@ struct autofs_fs_context {
>>    * context of the system call that passed the data and not the one 
>> that
>>    * triggered the superblock creation, lest the fd gets reassigned.
>>    */
>> -static int autofs_parse_fd(struct fs_context *fc, int fd)
>> +static int autofs_parse_fd(struct fs_context *fc, struct 
>> autofs_sb_info *sbi,
>> +                          struct fs_parameter *param,
>> +                          struct fs_parse_result *result)
>>   {
>> -       struct autofs_sb_info *sbi = fc->s_fs_info;
>>          struct file *pipe;
>>          int ret;
>>
>> -       pipe = fget(fd);
>> +       if (param->type == fs_value_is_file) {
>> +               /* came through the new api */
>> +               pipe = param->file;
>> +               param->file = NULL;
>> +       } else {
>> +               pipe = fget(result->uint_32);
>> +       }
>>          if (!pipe) {
>>                  errorf(fc, "could not open pipe file descriptor");
>>                  return -EBADF;
>> @@ -165,14 +172,15 @@ static int autofs_parse_fd(struct fs_context 
>> *fc, int fd)
>>          ret = autofs_check_pipe(pipe);
>>          if (ret < 0) {
>>                  errorf(fc, "Invalid/unusable pipe");
>> -               fput(pipe);
>> +               if (param->type != fs_value_is_file)
>> +                       fput(pipe);
>>                  return -EBADF;
>>          }
>>
>>          if (sbi->pipe)
>>                  fput(sbi->pipe);
>>
>> -       sbi->pipefd = fd;
>> +       sbi->pipefd = result->uint_32;
>>          sbi->pipe = pipe;
>>
>>          return 0;
>>
>> [2]:
>>  From 2f9171200505c82e744a235c85377e36ed190109 Mon Sep 17 00:00:00 2001
>> From: Christian Brauner <brauner@kernel.org>
>> Date: Fri, 22 Sep 2023 13:49:05 +0200
>> Subject: [PATCH] fsconfig: ensure that dirfd is set to aux
>>
>> The code in fs_param_is_fd() expects param->dirfd to be set to the fd
>> that was used to set param->file to initialize result->uint_32. So make
>> sure it's set so users like autofs using FSCONFIG_SET_FD with the new
>> mount api can rely on this to be set to the correct value.
>>
>> Signed-off-by: Christian Brauner <brauner@kernel.org>
>> ---
>>   fs/fsopen.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/fsopen.c b/fs/fsopen.c
>> index ce03f6521c88..6593ae518115 100644
>> --- a/fs/fsopen.c
>> +++ b/fs/fsopen.c
>> @@ -465,6 +465,7 @@ SYSCALL_DEFINE5(fsconfig,
>>           param.file = fget(aux);
>>           if (!param.file)
>>               goto out_key;
>> +        param.dirfd = aux;
>>           break;
>>       default:
>>           break;

[PATCH 8/8] autofs: fix protocol sub version setting

[Ian Kent]

There were a number of updates to protocol version 4, take account of
that when setting the super block info sub version field.

Signed-off-by: Ian Kent <raven@themaw.net>
---
 fs/autofs/inode.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/fs/autofs/inode.c b/fs/autofs/inode.c
index 3f2dfed428f9..53c0df354206 100644
--- a/fs/autofs/inode.c
+++ b/fs/autofs/inode.c
@@ -279,7 +279,17 @@ static int autofs_validate_protocol(struct fs_context *fc)
 		sbi->version = AUTOFS_MAX_PROTO_VERSION;
 	else
 		sbi->version = sbi->max_proto;
-	sbi->sub_version = AUTOFS_PROTO_SUBVERSION;
+
+	switch (sbi->version) {
+	case 4:
+		sbi->sub_version = 7;
+		break;
+	case 5:
+		sbi->sub_version = AUTOFS_PROTO_SUBVERSION;
+		break;
+	default:
+		sbi->sub_version = 0;
+	}
 
 	return 0;
 }
-- 
2.41.0

Re: [PATCH 0/8] autofs - convert to to use mount api

[Christian Brauner]

On Fri, 22 Sep 2023 12:12:07 +0800, Ian Kent wrote:
> There was a patch from David Howells to convert autofs to use the mount
> api but it was never merged.
> 
> I have taken David's patch and refactored it to make the change easier
> to review in the hope of having it merged.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>
> 
> [...]

Applied to the vfs.autofs branch of the vfs/vfs.git tree.
Patches in the vfs.autofs branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.autofs

[1/8] autofs: refactor autofs_prepare_pipe()
      https://git.kernel.org/vfs/vfs/c/a1388470620f
[2/8] autofs: add autofs_parse_fd()
      https://git.kernel.org/vfs/vfs/c/917c4dd6e625
[3/8] autofs: refactor super block info init
      https://git.kernel.org/vfs/vfs/c/81a57bf0af7c
[4/8] autofs: reformat 0pt enum declaration
      https://git.kernel.org/vfs/vfs/c/34539aa9def8
[5/8] autofs: refactor parse_options()
      https://git.kernel.org/vfs/vfs/c/805b2411ca1c
[6/8] autofs: validate protocol version
      https://git.kernel.org/vfs/vfs/c/89405b46e168
[7/8] autofs: convert autofs to use the new mount api
      https://git.kernel.org/vfs/vfs/c/7bf383b78c56
[8/8] autofs: fix protocol sub version setting
      https://git.kernel.org/vfs/vfs/c/10afd722e290

Re: [PATCH 0/8] autofs - convert to to use mount api

[Bill O'Donnell]

On Fri, Sep 22, 2023 at 12:12:07PM +0800, Ian Kent wrote:
> There was a patch from David Howells to convert autofs to use the mount
> api but it was never merged.
> 
> I have taken David's patch and refactored it to make the change easier
> to review in the hope of having it merged.
> 
> Signed-off-by: Ian Kent <raven@themaw.net>

for the series...
Reviewed-by: Bill O'Donnell <bodonnel@redhat.com>


> 
> Ian Kent (8):
>   autofs: refactor autofs_prepare_pipe()
>   autofs: add autofs_parse_fd()
>   autofs: refactor super block info init
>   autofs: reformat 0pt enum declaration
>   autofs: refactor parse_options()
>   autofs: validate protocol version
>   autofs: convert autofs to use the new mount api
>   autofs: fix protocol sub version setting
> 
>  fs/autofs/autofs_i.h |  15 +-
>  fs/autofs/init.c     |   9 +-
>  fs/autofs/inode.c    | 423 +++++++++++++++++++++++++------------------
>  3 files changed, 266 insertions(+), 181 deletions(-)
> 
> -- 
> 2.41.0
>