linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context
@ 2019-03-21 11:47 David Howells
  2019-03-21 11:47 ` [RFC PATCH 1/8] vfs: Add a single-or-reconfig keying to vfs_get_super() David Howells
                   ` (7 more replies)
  0 siblings, 8 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:47 UTC (permalink / raw)
  To: viro
  Cc: Colin Cross, Steven Rostedt, Tony Luck, Jeremy Kerr, Kees Cook,
	linux-s390, Heiko Carstens, Anton Vorontsov, Arnd Bergmann,
	Greg Kroah-Hartman, linuxppc-dev, Martin Schwidefsky,
	Rafael J. Wysocki, linux-fsdevel, linux-kernel, dhowells


Hi Al,

Here's a set of patches that converts the mount_single()-using filesystems
to use the new fs_context struct.  There may be prerequisite commits in the
branch detailed below.

 (1) Add a new keying to vfs_get_super() that indicates that
     ->reconfigure() should be called instead of (*fill_super)() if the
     superblock already exists.

 (2) Convert debugfs.

 (3) Convert tracefs.

 (4) Convert pstore.

 (5) Fix a bug in hypfs.

 (6) Convert hypfs.

 (7) Convert spufs.

 (8) Kill off mount_single().

These can be found in the following branch:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=mount-api-viro

Thanks,
David
---
David Howells (8):
      vfs: Add a single-or-reconfig keying to vfs_get_super()
      vfs: Convert debugfs to fs_context
      vfs: Convert tracefs to fs_context
      vfs: Convert pstore to fs_context
      hypfs: Fix error number left in struct pointer member
      vfs: Convert hypfs to fs_context
      vfs: Convert spufs to fs_context
      vfs: Kill off mount_single()


 Documentation/filesystems/vfs.txt         |    4 -
 arch/powerpc/platforms/cell/spufs/inode.c |  207 ++++++++++++++++-------------
 arch/s390/hypfs/inode.c                   |  137 +++++++++++--------
 fs/debugfs/inode.c                        |  186 ++++++++++++--------------
 fs/pstore/inode.c                         |  110 ++++++++++-----
 fs/super.c                                |   73 ++--------
 fs/tracefs/inode.c                        |  180 ++++++++++++-------------
 include/linux/fs.h                        |    3 
 include/linux/fs_context.h                |    1 
 9 files changed, 446 insertions(+), 455 deletions(-)


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

* [RFC PATCH 1/8] vfs: Add a single-or-reconfig keying to vfs_get_super()
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
@ 2019-03-21 11:47 ` David Howells
  2019-03-21 11:47 ` [RFC PATCH 2/8] vfs: Convert debugfs to fs_context David Howells
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:47 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-kernel, dhowells

Add an additional keying mode to vfs_get_super() to indicate that only a
single superblock should exist in the system, and that, if it does, further
mounts should invoke reconfiguration upon it.

This allows mount_single() to be replaced.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/super.c                 |   18 +++++++++++++-----
 include/linux/fs_context.h |    1 +
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/fs/super.c b/fs/super.c
index 6d8dbf309241..fa1d697b73bb 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1175,9 +1175,11 @@ int vfs_get_super(struct fs_context *fc,
 {
 	int (*test)(struct super_block *, struct fs_context *);
 	struct super_block *sb;
+	int err;
 
 	switch (keying) {
 	case vfs_get_single_super:
+	case vfs_get_single_reconf_super:
 		test = test_single_super;
 		break;
 	case vfs_get_keyed_super:
@@ -1195,18 +1197,24 @@ int vfs_get_super(struct fs_context *fc,
 		return PTR_ERR(sb);
 
 	if (!sb->s_root) {
-		int err = fill_super(sb, fc);
-		if (err) {
-			deactivate_locked_super(sb);
-			return err;
-		}
+		err = fill_super(sb, fc);
+		if (err)
+			goto error;
 
 		sb->s_flags |= SB_ACTIVE;
+	} else if (keying == vfs_get_single_reconf_super) {
+		err = reconfigure_super(fc);
+		if (err < 0)
+			goto error;
 	}
 
 	BUG_ON(fc->root);
 	fc->root = dget(sb->s_root);
 	return 0;
+
+error:
+	deactivate_locked_super(sb);
+	return err;
 }
 EXPORT_SYMBOL(vfs_get_super);
 
diff --git a/include/linux/fs_context.h b/include/linux/fs_context.h
index 8233c873af73..f1ed5ea0a041 100644
--- a/include/linux/fs_context.h
+++ b/include/linux/fs_context.h
@@ -136,6 +136,7 @@ extern int vfs_init_pseudo_fs_context(struct fs_context *fc,
  */
 enum vfs_get_super_keying {
 	vfs_get_single_super,	/* Only one such superblock may exist */
+	vfs_get_single_reconf_super, /* As above, but reconfigure if it exists */
 	vfs_get_keyed_super,	/* Superblocks with different s_fs_info keys may exist */
 	vfs_get_independent_super, /* Multiple independent superblocks may exist */
 };


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

* [RFC PATCH 2/8] vfs: Convert debugfs to fs_context
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
  2019-03-21 11:47 ` [RFC PATCH 1/8] vfs: Add a single-or-reconfig keying to vfs_get_super() David Howells
@ 2019-03-21 11:47 ` David Howells
  2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:47 UTC (permalink / raw)
  To: viro
  Cc: Greg Kroah-Hartman, Rafael J. Wysocki, linux-fsdevel,
	linux-kernel, dhowells

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
cc: "Rafael J. Wysocki" <rafael@kernel.org>
---

 fs/debugfs/inode.c |  186 ++++++++++++++++++++++++----------------------------
 1 file changed, 85 insertions(+), 101 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 95b5e78c22b1..951939bf25ce 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -12,6 +12,8 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/mount.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
 #include <linux/kobject.h>
@@ -20,7 +22,6 @@
 #include <linux/fsnotify.h>
 #include <linux/string.h>
 #include <linux/seq_file.h>
-#include <linux/parser.h>
 #include <linux/magic.h>
 #include <linux/slab.h>
 
@@ -43,73 +44,62 @@ static struct inode *debugfs_get_inode(struct super_block *sb)
 	return inode;
 }
 
-struct debugfs_mount_opts {
-	kuid_t uid;
-	kgid_t gid;
-	umode_t mode;
-};
-
 enum {
-	Opt_uid,
 	Opt_gid,
 	Opt_mode,
-	Opt_err
+	Opt_uid,
+};
+
+static const struct fs_parameter_spec debugfs_param_specs[] = {
+	fsparam_u32	("gid",				Opt_gid),
+	fsparam_u32oct	("mode",			Opt_mode),
+	fsparam_u32	("uid",				Opt_uid),
+	{}
 };
 
-static const match_table_t tokens = {
-	{Opt_uid, "uid=%u"},
-	{Opt_gid, "gid=%u"},
-	{Opt_mode, "mode=%o"},
-	{Opt_err, NULL}
+static const struct fs_parameter_description debugfs_fs_parameters = {
+	.name		= "debugfs",
+	.specs		= debugfs_param_specs,
 };
 
 struct debugfs_fs_info {
-	struct debugfs_mount_opts mount_opts;
+	kuid_t uid;
+	kgid_t gid;
+	umode_t mode;
 };
 
-static int debugfs_parse_options(char *data, struct debugfs_mount_opts *opts)
+static int debugfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	substring_t args[MAX_OPT_ARGS];
-	int option;
-	int token;
+	struct debugfs_fs_info *opts = fc->s_fs_info;
+	struct fs_parse_result result;
 	kuid_t uid;
 	kgid_t gid;
-	char *p;
-
-	opts->mode = DEBUGFS_DEFAULT_MODE;
-
-	while ((p = strsep(&data, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_uid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(uid))
-				return -EINVAL;
-			opts->uid = uid;
-			break;
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(gid))
-				return -EINVAL;
-			opts->gid = gid;
-			break;
-		case Opt_mode:
-			if (match_octal(&args[0], &option))
-				return -EINVAL;
-			opts->mode = option & S_IALLUGO;
-			break;
+	int opt;
+
+	opt = fs_parse(fc, &debugfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(uid))
+			return invalf(fc, "Unknown uid");
+		opts->uid = uid;
+		break;
+	case Opt_gid:
+		gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(gid))
+			return invalf(fc, "Unknown gid");
+		opts->gid = gid;
+		break;
+	case Opt_mode:
+		opts->mode = result.uint_32 & S_IALLUGO;
+		break;
 		/*
 		 * We might like to report bad mount options here;
 		 * but traditionally debugfs has ignored all mount options
 		 */
-		}
 	}
 
 	return 0;
@@ -119,46 +109,36 @@ static int debugfs_apply_options(struct super_block *sb)
 {
 	struct debugfs_fs_info *fsi = sb->s_fs_info;
 	struct inode *inode = d_inode(sb->s_root);
-	struct debugfs_mount_opts *opts = &fsi->mount_opts;
 
 	inode->i_mode &= ~S_IALLUGO;
-	inode->i_mode |= opts->mode;
+	inode->i_mode |= fsi->mode;
 
-	inode->i_uid = opts->uid;
-	inode->i_gid = opts->gid;
+	inode->i_uid = fsi->uid;
+	inode->i_gid = fsi->gid;
 
 	return 0;
 }
 
-static int debugfs_remount(struct super_block *sb, int *flags, char *data)
+static int debugfs_reconfigure(struct fs_context *fc)
 {
-	int err;
-	struct debugfs_fs_info *fsi = sb->s_fs_info;
+	struct super_block *sb = fc->root->d_sb;
 
 	sync_filesystem(sb);
-	err = debugfs_parse_options(data, &fsi->mount_opts);
-	if (err)
-		goto fail;
-
-	debugfs_apply_options(sb);
-
-fail:
-	return err;
+	return debugfs_apply_options(sb);
 }
 
 static int debugfs_show_options(struct seq_file *m, struct dentry *root)
 {
 	struct debugfs_fs_info *fsi = root->d_sb->s_fs_info;
-	struct debugfs_mount_opts *opts = &fsi->mount_opts;
 
-	if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
+	if (!uid_eq(fsi->uid, GLOBAL_ROOT_UID))
 		seq_printf(m, ",uid=%u",
-			   from_kuid_munged(&init_user_ns, opts->uid));
-	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
+			   from_kuid_munged(&init_user_ns, fsi->uid));
+	if (!gid_eq(fsi->gid, GLOBAL_ROOT_GID))
 		seq_printf(m, ",gid=%u",
-			   from_kgid_munged(&init_user_ns, opts->gid));
-	if (opts->mode != DEBUGFS_DEFAULT_MODE)
-		seq_printf(m, ",mode=%o", opts->mode);
+			   from_kgid_munged(&init_user_ns, fsi->gid));
+	if (fsi->mode != DEBUGFS_DEFAULT_MODE)
+		seq_printf(m, ",mode=%o", fsi->mode);
 
 	return 0;
 }
@@ -173,7 +153,6 @@ static void debugfs_evict_inode(struct inode *inode)
 
 static const struct super_operations debugfs_super_operations = {
 	.statfs		= simple_statfs,
-	.remount_fs	= debugfs_remount,
 	.show_options	= debugfs_show_options,
 	.evict_inode	= debugfs_evict_inode,
 };
@@ -199,51 +178,57 @@ static const struct dentry_operations debugfs_dops = {
 	.d_automount = debugfs_automount,
 };
 
-static int debug_fill_super(struct super_block *sb, void *data, int silent)
+static int debugfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	static const struct tree_descr debug_files[] = {{""}};
-	struct debugfs_fs_info *fsi;
 	int err;
 
-	fsi = kzalloc(sizeof(struct debugfs_fs_info), GFP_KERNEL);
-	sb->s_fs_info = fsi;
-	if (!fsi) {
-		err = -ENOMEM;
-		goto fail;
-	}
-
-	err = debugfs_parse_options(data, &fsi->mount_opts);
+	err = simple_fill_super(sb, DEBUGFS_MAGIC, debug_files);
 	if (err)
-		goto fail;
-
-	err  =  simple_fill_super(sb, DEBUGFS_MAGIC, debug_files);
-	if (err)
-		goto fail;
+		return err;
 
 	sb->s_op = &debugfs_super_operations;
 	sb->s_d_op = &debugfs_dops;
 
-	debugfs_apply_options(sb);
+	return debugfs_apply_options(sb);
+}
 
-	return 0;
+static int debugfs_get_tree(struct fs_context *fc)
+{
+	return vfs_get_super(fc, vfs_get_single_reconf_super,
+			     debugfs_fill_super);
+}
 
-fail:
-	kfree(fsi);
-	sb->s_fs_info = NULL;
-	return err;
+static void debugfs_free_fc(struct fs_context *fc)
+{
+	kfree(fc->s_fs_info);
 }
 
-static struct dentry *debug_mount(struct file_system_type *fs_type,
-			int flags, const char *dev_name,
-			void *data)
+static const struct fs_context_operations debugfs_context_ops = {
+	.free		= debugfs_free_fc,
+	.parse_param	= debugfs_parse_param,
+	.get_tree	= debugfs_get_tree,
+	.reconfigure	= debugfs_reconfigure,
+};
+
+static int debugfs_init_fs_context(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, debug_fill_super);
+	struct debugfs_fs_info *fsi;
+
+	fsi = kzalloc(sizeof(struct debugfs_fs_info), GFP_KERNEL);
+	if (!fsi)
+		return -ENOMEM;
+
+	fc->s_fs_info = fsi;
+	fc->ops = &debugfs_context_ops;
+	return 0;
 }
 
 static struct file_system_type debug_fs_type = {
 	.owner =	THIS_MODULE,
 	.name =		"debugfs",
-	.mount =	debug_mount,
+	.init_fs_context = debugfs_init_fs_context,
+	.parameters	= &debugfs_fs_parameters,
 	.kill_sb =	kill_litter_super,
 };
 MODULE_ALIAS_FS("debugfs");
@@ -862,4 +847,3 @@ static int __init debugfs_init(void)
 	return retval;
 }
 core_initcall(debugfs_init);
-


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

* [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
  2019-03-21 11:47 ` [RFC PATCH 1/8] vfs: Add a single-or-reconfig keying to vfs_get_super() David Howells
  2019-03-21 11:47 ` [RFC PATCH 2/8] vfs: Convert debugfs to fs_context David Howells
@ 2019-03-21 11:47 ` David Howells
  2019-03-21 12:58   ` Steven Rostedt
                     ` (2 more replies)
  2019-03-21 11:48 ` [RFC PATCH 4/8] vfs: Convert pstore " David Howells
                   ` (4 subsequent siblings)
  7 siblings, 3 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:47 UTC (permalink / raw)
  To: viro
  Cc: Steven Rostedt, Greg Kroah-Hartman, linux-fsdevel, linux-kernel,
	dhowells

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Steven Rostedt <rostedt@goodmis.org>
cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

 fs/tracefs/inode.c |  180 ++++++++++++++++++++++++----------------------------
 1 file changed, 83 insertions(+), 97 deletions(-)

diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 7098c49f3693..7ba64f38931f 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -16,12 +16,13 @@
 #include <linux/module.h>
 #include <linux/fs.h>
 #include <linux/mount.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/kobject.h>
 #include <linux/namei.h>
 #include <linux/tracefs.h>
 #include <linux/fsnotify.h>
 #include <linux/seq_file.h>
-#include <linux/parser.h>
 #include <linux/magic.h>
 #include <linux/slab.h>
 
@@ -138,73 +139,62 @@ static struct inode *tracefs_get_inode(struct super_block *sb)
 	return inode;
 }
 
-struct tracefs_mount_opts {
+struct tracefs_fs_info {
 	kuid_t uid;
 	kgid_t gid;
 	umode_t mode;
 };
 
 enum {
-	Opt_uid,
 	Opt_gid,
 	Opt_mode,
-	Opt_err
+	Opt_uid,
 };
 
-static const match_table_t tokens = {
-	{Opt_uid, "uid=%u"},
-	{Opt_gid, "gid=%u"},
-	{Opt_mode, "mode=%o"},
-	{Opt_err, NULL}
+static const struct fs_parameter_spec tracefs_param_specs[] = {
+	fsparam_u32	("gid",				Opt_gid),
+	fsparam_u32oct	("mode",			Opt_mode),
+	fsparam_u32	("uid",				Opt_uid),
+	{}
 };
 
-struct tracefs_fs_info {
-	struct tracefs_mount_opts mount_opts;
+static const struct fs_parameter_description tracefs_fs_parameters = {
+	.name		= "tracefs",
+	.specs		= tracefs_param_specs,
 };
 
-static int tracefs_parse_options(char *data, struct tracefs_mount_opts *opts)
+static int tracefs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	substring_t args[MAX_OPT_ARGS];
-	int option;
-	int token;
+	struct tracefs_fs_info *opts = fc->s_fs_info;
+	struct fs_parse_result result;
 	kuid_t uid;
 	kgid_t gid;
-	char *p;
-
-	opts->mode = TRACEFS_DEFAULT_MODE;
-
-	while ((p = strsep(&data, ",")) != NULL) {
-		if (!*p)
-			continue;
-
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_uid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(uid))
-				return -EINVAL;
-			opts->uid = uid;
-			break;
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(gid))
-				return -EINVAL;
-			opts->gid = gid;
-			break;
-		case Opt_mode:
-			if (match_octal(&args[0], &option))
-				return -EINVAL;
-			opts->mode = option & S_IALLUGO;
-			break;
+	int opt;
+
+	opt = fs_parse(fc, &tracefs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(uid))
+			return invalf(fc, "Unknown uid");
+		opts->uid = uid;
+		break;
+	case Opt_gid:
+		gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(gid))
+			return invalf(fc, "Unknown gid");
+		opts->gid = gid;
+		break;
+	case Opt_mode:
+		opts->mode = result.uint_32 & S_IALLUGO;
+		break;
 		/*
 		 * We might like to report bad mount options here;
 		 * but traditionally tracefs has ignored all mount options
 		 */
-		}
 	}
 
 	return 0;
@@ -214,100 +204,96 @@ static int tracefs_apply_options(struct super_block *sb)
 {
 	struct tracefs_fs_info *fsi = sb->s_fs_info;
 	struct inode *inode = sb->s_root->d_inode;
-	struct tracefs_mount_opts *opts = &fsi->mount_opts;
 
 	inode->i_mode &= ~S_IALLUGO;
-	inode->i_mode |= opts->mode;
+	inode->i_mode |= fsi->mode;
 
-	inode->i_uid = opts->uid;
-	inode->i_gid = opts->gid;
+	inode->i_uid = fsi->uid;
+	inode->i_gid = fsi->gid;
 
 	return 0;
 }
 
-static int tracefs_remount(struct super_block *sb, int *flags, char *data)
+static int tracefs_reconfigure(struct fs_context *fc)
 {
-	int err;
-	struct tracefs_fs_info *fsi = sb->s_fs_info;
+	struct super_block *sb = fc->root->d_sb;
 
 	sync_filesystem(sb);
-	err = tracefs_parse_options(data, &fsi->mount_opts);
-	if (err)
-		goto fail;
-
-	tracefs_apply_options(sb);
-
-fail:
-	return err;
+	return tracefs_apply_options(sb);
 }
 
 static int tracefs_show_options(struct seq_file *m, struct dentry *root)
 {
 	struct tracefs_fs_info *fsi = root->d_sb->s_fs_info;
-	struct tracefs_mount_opts *opts = &fsi->mount_opts;
 
-	if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
+	if (!uid_eq(fsi->uid, GLOBAL_ROOT_UID))
 		seq_printf(m, ",uid=%u",
-			   from_kuid_munged(&init_user_ns, opts->uid));
-	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
+			   from_kuid_munged(&init_user_ns, fsi->uid));
+	if (!gid_eq(fsi->gid, GLOBAL_ROOT_GID))
 		seq_printf(m, ",gid=%u",
-			   from_kgid_munged(&init_user_ns, opts->gid));
-	if (opts->mode != TRACEFS_DEFAULT_MODE)
-		seq_printf(m, ",mode=%o", opts->mode);
+			   from_kgid_munged(&init_user_ns, fsi->gid));
+	if (fsi->mode != TRACEFS_DEFAULT_MODE)
+		seq_printf(m, ",mode=%o", fsi->mode);
 
 	return 0;
 }
 
 static const struct super_operations tracefs_super_operations = {
 	.statfs		= simple_statfs,
-	.remount_fs	= tracefs_remount,
 	.show_options	= tracefs_show_options,
 };
 
-static int trace_fill_super(struct super_block *sb, void *data, int silent)
+static int tracefs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	static const struct tree_descr trace_files[] = {{""}};
-	struct tracefs_fs_info *fsi;
 	int err;
 
-	fsi = kzalloc(sizeof(struct tracefs_fs_info), GFP_KERNEL);
-	sb->s_fs_info = fsi;
-	if (!fsi) {
-		err = -ENOMEM;
-		goto fail;
-	}
-
-	err = tracefs_parse_options(data, &fsi->mount_opts);
+	err = simple_fill_super(sb, TRACEFS_MAGIC, trace_files);
 	if (err)
-		goto fail;
-
-	err  =  simple_fill_super(sb, TRACEFS_MAGIC, trace_files);
-	if (err)
-		goto fail;
+		return err;
 
 	sb->s_op = &tracefs_super_operations;
+	return tracefs_apply_options(sb);
+}
 
-	tracefs_apply_options(sb);
-
-	return 0;
+static int tracefs_get_tree(struct fs_context *fc)
+{
+	return vfs_get_super(fc, vfs_get_single_reconf_super,
+			     tracefs_fill_super);
+}
 
-fail:
-	kfree(fsi);
-	sb->s_fs_info = NULL;
-	return err;
+static void tracefs_free_fc(struct fs_context *fc)
+{
+	kfree(fc->s_fs_info);
 }
 
-static struct dentry *trace_mount(struct file_system_type *fs_type,
-			int flags, const char *dev_name,
-			void *data)
+static const struct fs_context_operations tracefs_context_ops = {
+	.free		= tracefs_free_fc,
+	.parse_param	= tracefs_parse_param,
+	.get_tree	= tracefs_get_tree,
+	.reconfigure	= tracefs_reconfigure,
+};
+
+static int tracefs_init_fs_context(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, trace_fill_super);
+	struct tracefs_fs_info *fsi;
+
+	fsi = kzalloc(sizeof(struct tracefs_fs_info), GFP_KERNEL);
+	if (!fsi)
+		return -ENOMEM;
+
+	fsi->mode = TRACEFS_DEFAULT_MODE;
+
+	fc->s_fs_info = fsi;
+	fc->ops = &tracefs_context_ops;
+	return 0;
 }
 
 static struct file_system_type trace_fs_type = {
 	.owner =	THIS_MODULE,
 	.name =		"tracefs",
-	.mount =	trace_mount,
+	.init_fs_context = tracefs_init_fs_context,
+	.parameters	= &tracefs_fs_parameters,
 	.kill_sb =	kill_litter_super,
 };
 MODULE_ALIAS_FS("tracefs");


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

* [RFC PATCH 4/8] vfs: Convert pstore to fs_context
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
                   ` (2 preceding siblings ...)
  2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
@ 2019-03-21 11:48 ` David Howells
  2019-03-21 16:31   ` Kees Cook
  2019-03-21 17:03   ` David Howells
  2019-03-21 11:48 ` [RFC PATCH 5/8] hypfs: Fix error number left in struct pointer member David Howells
                   ` (3 subsequent siblings)
  7 siblings, 2 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:48 UTC (permalink / raw)
  To: viro
  Cc: Kees Cook, Anton Vorontsov, Colin Cross, Tony Luck,
	linux-fsdevel, linux-kernel, dhowells

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Kees Cook <keescook@chromium.org>
cc: Anton Vorontsov <anton@enomsg.org>
cc: Colin Cross <ccross@android.com>
cc: Tony Luck <tony.luck@intel.com>
---

 fs/pstore/inode.c |  110 ++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 71 insertions(+), 39 deletions(-)

diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c
index c60ee46f3e39..4625246fa992 100644
--- a/fs/pstore/inode.c
+++ b/fs/pstore/inode.c
@@ -19,6 +19,8 @@
 
 #include <linux/module.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/fsnotify.h>
 #include <linux/pagemap.h>
 #include <linux/highmem.h>
@@ -26,10 +28,8 @@
 #include <linux/init.h>
 #include <linux/list.h>
 #include <linux/string.h>
-#include <linux/mount.h>
 #include <linux/seq_file.h>
 #include <linux/ramfs.h>
-#include <linux/parser.h>
 #include <linux/sched.h>
 #include <linux/magic.h>
 #include <linux/pstore.h>
@@ -227,38 +227,48 @@ static struct inode *pstore_get_inode(struct super_block *sb)
 	return inode;
 }
 
+struct pstore_fs_context {
+	unsigned int	kmsg_bytes;
+};
+
 enum {
-	Opt_kmsg_bytes, Opt_err
+	Opt_kmsg_bytes,
 };
 
-static const match_table_t tokens = {
-	{Opt_kmsg_bytes, "kmsg_bytes=%u"},
-	{Opt_err, NULL}
+static const struct fs_parameter_spec pstore_param_specs[] = {
+	fsparam_u32	("kmsg_bytes",		Opt_kmsg_bytes),
+	{}
 };
 
-static void parse_options(char *options)
-{
-	char		*p;
-	substring_t	args[MAX_OPT_ARGS];
-	int		option;
+static const struct fs_parameter_description pstore_fs_parameters = {
+	.name		= "pstore",
+	.specs		= pstore_param_specs,
+};
 
-	if (!options)
-		return;
+static int pstore_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct pstore_fs_context *ctx = fc->fs_private;
+	struct fs_parse_result result;
+	int opt;
+
+	opt = fs_parse(fc, &pstore_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_kmsg_bytes:
+		ctx->kmsg_bytes = result.uint_32;
+		break;
+	}
 
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token;
+	return 0;
+}
 
-		if (!*p)
-			continue;
+static void pstore_apply_param(struct fs_context *fc)
+{
+	struct pstore_fs_context *ctx = fc->fs_private;
 
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_kmsg_bytes:
-			if (!match_int(&args[0], &option))
-				pstore_set_kmsg_bytes(option);
-			break;
-		}
-	}
+	pstore_set_kmsg_bytes(ctx->kmsg_bytes);
 }
 
 /*
@@ -271,11 +281,10 @@ static int pstore_show_options(struct seq_file *m, struct dentry *root)
 	return 0;
 }
 
-static int pstore_remount(struct super_block *sb, int *flags, char *data)
+static int pstore_reconfigure(struct fs_context *fc)
 {
-	sync_filesystem(sb);
-	parse_options(data);
-
+	sync_filesystem(fc->root->d_sb);
+	pstore_apply_param(fc);
 	return 0;
 }
 
@@ -283,7 +292,6 @@ static const struct super_operations pstore_ops = {
 	.statfs		= simple_statfs,
 	.drop_inode	= generic_delete_inode,
 	.evict_inode	= pstore_evict_inode,
-	.remount_fs	= pstore_remount,
 	.show_options	= pstore_show_options,
 };
 
@@ -389,12 +397,10 @@ void pstore_get_records(int quiet)
 	inode_unlock(d_inode(root));
 }
 
-static int pstore_fill_super(struct super_block *sb, void *data, int silent)
+static int pstore_fill_super(struct super_block *sb, struct fs_context *fc)
 {
 	struct inode *inode;
 
-	pstore_sb = sb;
-
 	sb->s_maxbytes		= MAX_LFS_FILESIZE;
 	sb->s_blocksize		= PAGE_SIZE;
 	sb->s_blocksize_bits	= PAGE_SHIFT;
@@ -402,7 +408,8 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_op		= &pstore_ops;
 	sb->s_time_gran		= 1;
 
-	parse_options(data);
+	pstore_sb = sb;
+	pstore_apply_param(fc);
 
 	inode = pstore_get_inode(sb);
 	if (inode) {
@@ -416,14 +423,38 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
 		return -ENOMEM;
 
 	pstore_get_records(0);
-
 	return 0;
 }
 
-static struct dentry *pstore_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static int pstore_get_tree(struct fs_context *fc)
+{
+	return vfs_get_super(fc, vfs_get_single_reconf_super,
+			     pstore_fill_super);
+}
+
+static void pstore_free_fc(struct fs_context *fc)
 {
-	return mount_single(fs_type, flags, data, pstore_fill_super);
+	kfree(fc->fs_private);
+}
+
+static const struct fs_context_operations pstore_context_ops = {
+	.free		= pstore_free_fc,
+	.parse_param	= pstore_parse_param,
+	.get_tree	= pstore_get_tree,
+	.reconfigure	= pstore_reconfigure,
+};
+
+static int pstore_init_fs_context(struct fs_context *fc)
+{
+	struct pstore_fs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct pstore_fs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	fc->fs_private = ctx;
+	fc->ops = &pstore_context_ops;
+	return 0;
 }
 
 static void pstore_kill_sb(struct super_block *sb)
@@ -435,7 +466,8 @@ static void pstore_kill_sb(struct super_block *sb)
 static struct file_system_type pstore_fs_type = {
 	.owner          = THIS_MODULE,
 	.name		= "pstore",
-	.mount		= pstore_mount,
+	.init_fs_context = pstore_init_fs_context,
+	.parameters	= &pstore_fs_parameters,
 	.kill_sb	= pstore_kill_sb,
 };
 


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

* [RFC PATCH 5/8] hypfs: Fix error number left in struct pointer member
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
                   ` (3 preceding siblings ...)
  2019-03-21 11:48 ` [RFC PATCH 4/8] vfs: Convert pstore " David Howells
@ 2019-03-21 11:48 ` David Howells
  2019-03-21 11:48 ` [RFC PATCH 6/8] vfs: Convert hypfs to fs_context David Howells
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:48 UTC (permalink / raw)
  To: viro
  Cc: Martin Schwidefsky, Heiko Carstens, linux-s390, linux-fsdevel,
	linux-kernel, dhowells

In hypfs_fill_super(), if hypfs_create_update_file() fails,
sbi->update_file is left holding an error number.  This is passed to
hypfs_kill_super() which doesn't check for this.

Fix this by not setting sbi->update_value until after we've checked for
error.

Fixes: 24bbb1faf3f0 ("[PATCH] s390_hypfs filesystem")
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
cc: Heiko Carstens <heiko.carstens@de.ibm.com>
cc: linux-s390@vger.kernel.org
---

 arch/s390/hypfs/inode.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index ccad1398abd4..b5cfcad953c2 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -269,7 +269,7 @@ static int hypfs_show_options(struct seq_file *s, struct dentry *root)
 static int hypfs_fill_super(struct super_block *sb, void *data, int silent)
 {
 	struct inode *root_inode;
-	struct dentry *root_dentry;
+	struct dentry *root_dentry, *update_file;
 	int rc = 0;
 	struct hypfs_sb_info *sbi;
 
@@ -300,9 +300,10 @@ static int hypfs_fill_super(struct super_block *sb, void *data, int silent)
 		rc = hypfs_diag_create_files(root_dentry);
 	if (rc)
 		return rc;
-	sbi->update_file = hypfs_create_update_file(root_dentry);
-	if (IS_ERR(sbi->update_file))
-		return PTR_ERR(sbi->update_file);
+	update_file = hypfs_create_update_file(root_dentry);
+	if (IS_ERR(update_file))
+		return PTR_ERR(update_file);
+	sbi->update_file = update_file;
 	hypfs_update_update(sb);
 	pr_info("Hypervisor filesystem mounted\n");
 	return 0;


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

* [RFC PATCH 6/8] vfs: Convert hypfs to fs_context
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
                   ` (4 preceding siblings ...)
  2019-03-21 11:48 ` [RFC PATCH 5/8] hypfs: Fix error number left in struct pointer member David Howells
@ 2019-03-21 11:48 ` David Howells
  2019-03-21 11:48 ` [RFC PATCH 7/8] vfs: Convert spufs " David Howells
  2019-03-21 11:48 ` [RFC PATCH 8/8] vfs: Kill off mount_single() David Howells
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:48 UTC (permalink / raw)
  To: viro
  Cc: Martin Schwidefsky, Heiko Carstens, linux-s390, linux-fsdevel,
	linux-kernel, dhowells

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
cc: Heiko Carstens <heiko.carstens@de.ibm.com>
cc: linux-s390@vger.kernel.org
---

 arch/s390/hypfs/inode.c |  128 ++++++++++++++++++++++++++---------------------
 1 file changed, 70 insertions(+), 58 deletions(-)

diff --git a/arch/s390/hypfs/inode.c b/arch/s390/hypfs/inode.c
index b5cfcad953c2..d1b5f75334af 100644
--- a/arch/s390/hypfs/inode.c
+++ b/arch/s390/hypfs/inode.c
@@ -12,17 +12,17 @@
 #include <linux/types.h>
 #include <linux/errno.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/namei.h>
 #include <linux/vfs.h>
 #include <linux/slab.h>
 #include <linux/pagemap.h>
 #include <linux/time.h>
-#include <linux/parser.h>
 #include <linux/sysfs.h>
 #include <linux/init.h>
 #include <linux/kobject.h>
 #include <linux/seq_file.h>
-#include <linux/mount.h>
 #include <linux/uio.h>
 #include <asm/ebcdic.h>
 #include "hypfs.h"
@@ -207,52 +207,44 @@ static int hypfs_release(struct inode *inode, struct file *filp)
 	return 0;
 }
 
-enum { opt_uid, opt_gid, opt_err };
+enum { Opt_uid, Opt_gid, };
 
-static const match_table_t hypfs_tokens = {
-	{opt_uid, "uid=%u"},
-	{opt_gid, "gid=%u"},
-	{opt_err, NULL}
+static const struct fs_parameter_spec hypfs_param_specs[] = {
+	fsparam_u32("gid", Opt_gid),
+	fsparam_u32("uid", Opt_uid),
+	{}
 };
 
-static int hypfs_parse_options(char *options, struct super_block *sb)
+static const struct fs_parameter_description hypfs_fs_parameters = {
+	.name		= "hypfs",
+	.specs		= hypfs_param_specs,
+};
+
+static int hypfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
 {
-	char *str;
-	substring_t args[MAX_OPT_ARGS];
+	struct hypfs_sb_info *hypfs_info = fc->s_fs_info;
+	struct fs_parse_result result;
 	kuid_t uid;
 	kgid_t gid;
-
-	if (!options)
-		return 0;
-	while ((str = strsep(&options, ",")) != NULL) {
-		int token, option;
-		struct hypfs_sb_info *hypfs_info = sb->s_fs_info;
-
-		if (!*str)
-			continue;
-		token = match_token(str, hypfs_tokens, args);
-		switch (token) {
-		case opt_uid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(uid))
-				return -EINVAL;
-			hypfs_info->uid = uid;
-			break;
-		case opt_gid:
-			if (match_int(&args[0], &option))
-				return -EINVAL;
-			gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(gid))
-				return -EINVAL;
-			hypfs_info->gid = gid;
-			break;
-		case opt_err:
-		default:
-			pr_err("%s is not a valid mount option\n", str);
-			return -EINVAL;
-		}
+	int opt;
+
+	opt = fs_parse(fc, &hypfs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(uid))
+			return invalf(fc, "Unknown uid");
+		hypfs_info->uid = uid;
+		break;
+	case Opt_gid:
+		gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(gid))
+			return invalf(fc, "Unknown gid");
+		hypfs_info->gid = gid;
+		break;
 	}
 	return 0;
 }
@@ -266,26 +258,18 @@ static int hypfs_show_options(struct seq_file *s, struct dentry *root)
 	return 0;
 }
 
-static int hypfs_fill_super(struct super_block *sb, void *data, int silent)
+static int hypfs_fill_super(struct super_block *sb, struct fs_context *fc)
 {
+	struct hypfs_sb_info *sbi = sb->s_fs_info;
 	struct inode *root_inode;
 	struct dentry *root_dentry, *update_file;
-	int rc = 0;
-	struct hypfs_sb_info *sbi;
+	int rc;
 
-	sbi = kzalloc(sizeof(struct hypfs_sb_info), GFP_KERNEL);
-	if (!sbi)
-		return -ENOMEM;
-	mutex_init(&sbi->lock);
-	sbi->uid = current_uid();
-	sbi->gid = current_gid();
-	sb->s_fs_info = sbi;
 	sb->s_blocksize = PAGE_SIZE;
 	sb->s_blocksize_bits = PAGE_SHIFT;
 	sb->s_magic = HYPFS_MAGIC;
 	sb->s_op = &hypfs_s_ops;
-	if (hypfs_parse_options(data, sb))
-		return -EINVAL;
+
 	root_inode = hypfs_make_inode(sb, S_IFDIR | 0755);
 	if (!root_inode)
 		return -ENOMEM;
@@ -309,10 +293,37 @@ static int hypfs_fill_super(struct super_block *sb, void *data, int silent)
 	return 0;
 }
 
-static struct dentry *hypfs_mount(struct file_system_type *fst, int flags,
-			const char *devname, void *data)
+static int hypfs_get_tree(struct fs_context *fc)
+{
+	return vfs_get_super(fc, vfs_get_single_super, hypfs_fill_super);
+}
+
+static void hypfs_free_fc(struct fs_context *fc)
 {
-	return mount_single(fst, flags, data, hypfs_fill_super);
+	kfree(fc->s_fs_info);
+}
+
+static const struct fs_context_operations hypfs_context_ops = {
+	.free		= hypfs_free_fc,
+	.parse_param	= hypfs_parse_param,
+	.get_tree	= hypfs_get_tree,
+};
+
+static int hypfs_init_fs_context(struct fs_context *fc)
+{
+	struct hypfs_sb_info *sbi;
+
+	sbi = kzalloc(sizeof(struct hypfs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		return -ENOMEM;
+
+	mutex_init(&sbi->lock);
+	sbi->uid = current_uid();
+	sbi->gid = current_gid();
+
+	fc->s_fs_info = sbi;
+	fc->ops = &hypfs_context_ops;
+	return 0;
 }
 
 static void hypfs_kill_super(struct super_block *sb)
@@ -443,7 +454,8 @@ static const struct file_operations hypfs_file_ops = {
 static struct file_system_type hypfs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "s390_hypfs",
-	.mount		= hypfs_mount,
+	.init_fs_context = hypfs_init_fs_context,
+	.parameters	= &hypfs_fs_parameters,
 	.kill_sb	= hypfs_kill_super
 };
 


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

* [RFC PATCH 7/8] vfs: Convert spufs to fs_context
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
                   ` (5 preceding siblings ...)
  2019-03-21 11:48 ` [RFC PATCH 6/8] vfs: Convert hypfs to fs_context David Howells
@ 2019-03-21 11:48 ` David Howells
  2019-03-21 11:48 ` [RFC PATCH 8/8] vfs: Kill off mount_single() David Howells
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:48 UTC (permalink / raw)
  To: viro
  Cc: Jeremy Kerr, Arnd Bergmann, linuxppc-dev, linux-fsdevel,
	linux-kernel, dhowells

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Jeremy Kerr <jk@ozlabs.org>
cc: Arnd Bergmann <arnd@arndb.de>
cc: linuxppc-dev@lists.ozlabs.org
---

 arch/powerpc/platforms/cell/spufs/inode.c |  207 ++++++++++++++++-------------
 1 file changed, 116 insertions(+), 91 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/inode.c b/arch/powerpc/platforms/cell/spufs/inode.c
index db329d4bf1c3..f951a7fe4e3c 100644
--- a/arch/powerpc/platforms/cell/spufs/inode.c
+++ b/arch/powerpc/platforms/cell/spufs/inode.c
@@ -23,6 +23,8 @@
 
 #include <linux/file.h>
 #include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/fs_parser.h>
 #include <linux/fsnotify.h>
 #include <linux/backing-dev.h>
 #include <linux/init.h>
@@ -33,7 +35,6 @@
 #include <linux/pagemap.h>
 #include <linux/poll.h>
 #include <linux/slab.h>
-#include <linux/parser.h>
 
 #include <asm/prom.h>
 #include <asm/spu.h>
@@ -43,7 +44,7 @@
 #include "spufs.h"
 
 struct spufs_sb_info {
-	int debug;
+	bool debug;
 };
 
 static struct kmem_cache *spufs_inode_cache;
@@ -593,16 +594,27 @@ long spufs_create(struct path *path, struct dentry *dentry,
 }
 
 /* File system initialization */
+struct spufs_fs_context {
+	kuid_t	uid;
+	kgid_t	gid;
+	umode_t	mode;
+};
+
 enum {
-	Opt_uid, Opt_gid, Opt_mode, Opt_debug, Opt_err,
+	Opt_uid, Opt_gid, Opt_mode, Opt_debug,
+};
+
+static const struct fs_parameter_spec spufs_param_specs[] = {
+	fsparam_u32	("gid",				Opt_gid),
+	fsparam_u32oct	("mode",			Opt_mode),
+	fsparam_u32	("uid",				Opt_uid),
+	fsparam_flag	("debug",			Opt_debug),
+	{}
 };
 
-static const match_table_t spufs_tokens = {
-	{ Opt_uid,   "uid=%d" },
-	{ Opt_gid,   "gid=%d" },
-	{ Opt_mode,  "mode=%o" },
-	{ Opt_debug, "debug" },
-	{ Opt_err,    NULL  },
+static const struct fs_parameter_description spufs_fs_parameters = {
+	.name		= "spufs",
+	.specs		= spufs_param_specs,
 };
 
 static int spufs_show_options(struct seq_file *m, struct dentry *root)
@@ -623,47 +635,41 @@ static int spufs_show_options(struct seq_file *m, struct dentry *root)
 	return 0;
 }
 
-static int
-spufs_parse_options(struct super_block *sb, char *options, struct inode *root)
-{
-	char *p;
-	substring_t args[MAX_OPT_ARGS];
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token, option;
-
-		if (!*p)
-			continue;
-
-		token = match_token(p, spufs_tokens, args);
-		switch (token) {
-		case Opt_uid:
-			if (match_int(&args[0], &option))
-				return 0;
-			root->i_uid = make_kuid(current_user_ns(), option);
-			if (!uid_valid(root->i_uid))
-				return 0;
-			break;
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return 0;
-			root->i_gid = make_kgid(current_user_ns(), option);
-			if (!gid_valid(root->i_gid))
-				return 0;
-			break;
-		case Opt_mode:
-			if (match_octal(&args[0], &option))
-				return 0;
-			root->i_mode = option | S_IFDIR;
-			break;
-		case Opt_debug:
-			spufs_get_sb_info(sb)->debug = 1;
-			break;
-		default:
-			return 0;
-		}
+static int spufs_parse_param(struct fs_context *fc, struct fs_parameter *param)
+{
+	struct spufs_fs_context *ctx = fc->fs_private;
+	struct spufs_sb_info *sbi = fc->s_fs_info;
+	struct fs_parse_result result;
+	kuid_t uid;
+	kgid_t gid;
+	int opt;
+
+	opt = fs_parse(fc, &spufs_fs_parameters, param, &result);
+	if (opt < 0)
+		return opt;
+
+	switch (opt) {
+	case Opt_uid:
+		uid = make_kuid(current_user_ns(), result.uint_32);
+		if (!uid_valid(uid))
+			return invalf(fc, "Unknown uid");
+		ctx->uid = uid;
+		break;
+	case Opt_gid:
+		gid = make_kgid(current_user_ns(), result.uint_32);
+		if (!gid_valid(gid))
+			return invalf(fc, "Unknown gid");
+		ctx->gid = gid;
+		break;
+	case Opt_mode:
+		ctx->mode = result.uint_32 & S_IALLUGO;
+		break;
+	case Opt_debug:
+		sbi->debug = true;
+		break;
 	}
-	return 1;
+
+	return 0;
 }
 
 static void spufs_exit_isolated_loader(void)
@@ -697,79 +703,98 @@ spufs_init_isolated_loader(void)
 	printk(KERN_INFO "spufs: SPU isolation mode enabled\n");
 }
 
-static int
-spufs_create_root(struct super_block *sb, void *data)
+static int spufs_create_root(struct super_block *sb, struct fs_context *fc)
 {
+	struct spufs_fs_context *ctx = fc->fs_private;
 	struct inode *inode;
-	int ret;
 
-	ret = -ENODEV;
 	if (!spu_management_ops)
-		goto out;
+		return -ENODEV;
 
-	ret = -ENOMEM;
-	inode = spufs_new_inode(sb, S_IFDIR | 0775);
+	inode = spufs_new_inode(sb, S_IFDIR | ctx->mode);
 	if (!inode)
-		goto out;
+		return -ENOMEM;
 
+	inode->i_uid = ctx->uid;
+	inode->i_gid = ctx->gid;
 	inode->i_op = &simple_dir_inode_operations;
 	inode->i_fop = &simple_dir_operations;
 	SPUFS_I(inode)->i_ctx = NULL;
 	inc_nlink(inode);
 
-	ret = -EINVAL;
-	if (!spufs_parse_options(sb, data, inode))
-		goto out_iput;
-
-	ret = -ENOMEM;
 	sb->s_root = d_make_root(inode);
 	if (!sb->s_root)
-		goto out;
-
+		return -ENOMEM;
 	return 0;
-out_iput:
-	iput(inode);
-out:
-	return ret;
 }
 
-static int
-spufs_fill_super(struct super_block *sb, void *data, int silent)
-{
-	struct spufs_sb_info *info;
-	static const struct super_operations s_ops = {
-		.alloc_inode = spufs_alloc_inode,
-		.destroy_inode = spufs_destroy_inode,
-		.statfs = simple_statfs,
-		.evict_inode = spufs_evict_inode,
-		.show_options = spufs_show_options,
-	};
-
-	info = kzalloc(sizeof(*info), GFP_KERNEL);
-	if (!info)
-		return -ENOMEM;
+static const struct super_operations spufs_ops = {
+	.alloc_inode	= spufs_alloc_inode,
+	.destroy_inode	= spufs_destroy_inode,
+	.statfs		= simple_statfs,
+	.evict_inode	= spufs_evict_inode,
+	.show_options	= spufs_show_options,
+};
 
+static int spufs_fill_super(struct super_block *sb, struct fs_context *fc)
+{
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_blocksize = PAGE_SIZE;
 	sb->s_blocksize_bits = PAGE_SHIFT;
 	sb->s_magic = SPUFS_MAGIC;
-	sb->s_op = &s_ops;
-	sb->s_fs_info = info;
+	sb->s_op = &spufs_ops;
 
-	return spufs_create_root(sb, data);
+	return spufs_create_root(sb, fc);
+}
+
+static int spufs_get_tree(struct fs_context *fc)
+{
+	return vfs_get_super(fc, vfs_get_single_super, spufs_fill_super);
 }
 
-static struct dentry *
-spufs_mount(struct file_system_type *fstype, int flags,
-		const char *name, void *data)
+static void spufs_free_fc(struct fs_context *fc)
 {
-	return mount_single(fstype, flags, data, spufs_fill_super);
+	kfree(fc->s_fs_info);
+}
+
+static const struct fs_context_operations spufs_context_ops = {
+	.free		= spufs_free_fc,
+	.parse_param	= spufs_parse_param,
+	.get_tree	= spufs_get_tree,
+};
+
+static int spufs_init_fs_context(struct fs_context *fc)
+{
+	struct spufs_fs_context *ctx;
+	struct spufs_sb_info *sbi;
+
+	ctx = kzalloc(sizeof(struct spufs_fs_context), GFP_KERNEL);
+	if (!ctx)
+		goto nomem;
+
+	sbi = kzalloc(sizeof(struct spufs_sb_info), GFP_KERNEL);
+	if (!sbi)
+		goto nomem_ctx;
+
+	ctx->uid = current_uid();
+	ctx->gid = current_gid();
+	ctx->mode = 0755;
+
+	fc->s_fs_info = sbi;
+	fc->ops = &spufs_context_ops;
+	return 0;
+
+nomem_ctx:
+	kfree(ctx);
+nomem:
+	return -ENOMEM;
 }
 
 static struct file_system_type spufs_type = {
 	.owner = THIS_MODULE,
 	.name = "spufs",
-	.mount = spufs_mount,
+	.init_fs_context = spufs_init_fs_context,
+	.parameters	= &spufs_fs_parameters,
 	.kill_sb = kill_litter_super,
 };
 MODULE_ALIAS_FS("spufs");


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

* [RFC PATCH 8/8] vfs: Kill off mount_single()
  2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
                   ` (6 preceding siblings ...)
  2019-03-21 11:48 ` [RFC PATCH 7/8] vfs: Convert spufs " David Howells
@ 2019-03-21 11:48 ` David Howells
  7 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 11:48 UTC (permalink / raw)
  To: viro; +Cc: linux-fsdevel, linux-kernel, dhowells

Kill mount_single() off as nothing now uses it.  It has been replaced by
vfs_get_super() keyed with vfs_get_single_super or
vfs_get_single_reconf_super.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 Documentation/filesystems/vfs.txt |    4 +--
 fs/super.c                        |   55 -------------------------------------
 include/linux/fs.h                |    3 --
 3 files changed, 2 insertions(+), 60 deletions(-)

diff --git a/Documentation/filesystems/vfs.txt b/Documentation/filesystems/vfs.txt
index 761c6fd24a53..6ef83fdfffdc 100644
--- a/Documentation/filesystems/vfs.txt
+++ b/Documentation/filesystems/vfs.txt
@@ -181,8 +181,8 @@ and provides a fill_super() callback instead. The generic variants are:
 
   mount_nodev: mount a filesystem that is not backed by a device
 
-  mount_single: mount a filesystem which shares the instance between
-  	all mounts
+  vfs_get_super: mount a filesystem with one of a number of superblock
+		 sharing options.
 
 A fill_super() callback implementation has the following arguments:
 
diff --git a/fs/super.c b/fs/super.c
index fa1d697b73bb..39963e597118 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -1449,61 +1449,6 @@ struct dentry *mount_nodev(struct file_system_type *fs_type,
 }
 EXPORT_SYMBOL(mount_nodev);
 
-static int reconfigure_single(struct super_block *s,
-			      int flags, void *data)
-{
-	struct fs_context *fc;
-	int ret;
-
-	/* The caller really need to be passing fc down into mount_single(),
-	 * then a chunk of this can be removed.  [Bollocks -- AV]
-	 * Better yet, reconfiguration shouldn't happen, but rather the second
-	 * mount should be rejected if the parameters are not compatible.
-	 */
-	fc = fs_context_for_reconfigure(s->s_root, flags, MS_RMT_MASK);
-	if (IS_ERR(fc))
-		return PTR_ERR(fc);
-
-	ret = parse_monolithic_mount_data(fc, data);
-	if (ret < 0)
-		goto out;
-
-	ret = reconfigure_super(fc);
-out:
-	put_fs_context(fc);
-	return ret;
-}
-
-static int compare_single(struct super_block *s, void *p)
-{
-	return 1;
-}
-
-struct dentry *mount_single(struct file_system_type *fs_type,
-	int flags, void *data,
-	int (*fill_super)(struct super_block *, void *, int))
-{
-	struct super_block *s;
-	int error;
-
-	s = sget(fs_type, compare_single, set_anon_super, flags, NULL);
-	if (IS_ERR(s))
-		return ERR_CAST(s);
-	if (!s->s_root) {
-		error = fill_super(s, data, flags & SB_SILENT ? 1 : 0);
-		if (!error)
-			s->s_flags |= SB_ACTIVE;
-	} else {
-		error = reconfigure_single(s, flags, data);
-	}
-	if (unlikely(error)) {
-		deactivate_locked_super(s);
-		return ERR_PTR(error);
-	}
-	return dget(s->s_root);
-}
-EXPORT_SYMBOL(mount_single);
-
 /**
  * vfs_get_tree - Get the mountable root
  * @fc: The superblock configuration context.
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 646f4ccaeee9..5ca14600dcf6 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2206,9 +2206,6 @@ static inline struct dentry *mount_bdev(struct file_system_type *fs_type,
 	return ERR_PTR(-ENODEV);
 }
 #endif
-extern struct dentry *mount_single(struct file_system_type *fs_type,
-	int flags, void *data,
-	int (*fill_super)(struct super_block *, void *, int));
 extern struct dentry *mount_nodev(struct file_system_type *fs_type,
 	int flags, void *data,
 	int (*fill_super)(struct super_block *, void *, int));


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

* Re: [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
@ 2019-03-21 12:58   ` Steven Rostedt
  2019-03-21 15:49   ` David Howells
  2019-03-21 15:50   ` David Howells
  2 siblings, 0 replies; 17+ messages in thread
From: Steven Rostedt @ 2019-03-21 12:58 UTC (permalink / raw)
  To: David Howells; +Cc: viro, Greg Kroah-Hartman, linux-fsdevel, linux-kernel

On Thu, 21 Mar 2019 11:47:57 +0000
David Howells <dhowells@redhat.com> wrote:

Very weak change log (none!). What is this fs_context, and why does it
need to change? Each patch should hold its own as a stand alone, as git
history shows a patch not a series, and the cover letters will get lost
over time.

-- Steve


> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Steven Rostedt <rostedt@goodmis.org>
> cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
> 
>  fs/tracefs/inode.c |  180 ++++++++++++++++++++++++----------------------------
>  1 file changed, 83 insertions(+), 97 deletions(-)
> 
> diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
> index 7098c49f3693..7ba64f38931f 100644
> --- a/fs/tracefs/inode.c
> +++ b/fs/tracefs/inode.c
> @@ -16,12 +16,13 @@
>  #include <linux/module.h>
>  #include <linux/fs.h>
>  #include <linux/mount.h>
> +#include <linux/fs_context.h>
> +#include <linux/fs_parser.h>
>  #include <linux/kobject.h>
>  #include <linux/namei.h>
>  #include <linux/tracefs.h>
>  #include <linux/fsnotify.h>
>  #include <linux/seq_file.h>
> -#include <linux/parser.h>
>  #include <linux/magic.h>
>  #include <linux/slab.h>
>  
> @@ -138,73 +139,62 @@ static struct inode *tracefs_get_inode(struct super_block *sb)
>  	return inode;
>  }
>  
> -struct tracefs_mount_opts {
> +struct tracefs_fs_info {
>  	kuid_t uid;
>  	kgid_t gid;
>  	umode_t mode;
>  };
>  
>  enum {
> -	Opt_uid,
>  	Opt_gid,
>  	Opt_mode,
> -	Opt_err
> +	Opt_uid,
>  };
>  
> -static const match_table_t tokens = {
> -	{Opt_uid, "uid=%u"},
> -	{Opt_gid, "gid=%u"},
> -	{Opt_mode, "mode=%o"},
> -	{Opt_err, NULL}
> +static const struct fs_parameter_spec tracefs_param_specs[] = {
> +	fsparam_u32	("gid",				Opt_gid),
> +	fsparam_u32oct	("mode",			Opt_mode),
> +	fsparam_u32	("uid",				Opt_uid),
> +	{}
>  };
>  
> -struct tracefs_fs_info {
> -	struct tracefs_mount_opts mount_opts;
> +static const struct fs_parameter_description tracefs_fs_parameters = {
> +	.name		= "tracefs",
> +	.specs		= tracefs_param_specs,
>  };
>  
> -static int tracefs_parse_options(char *data, struct tracefs_mount_opts *opts)
> +static int tracefs_parse_param(struct fs_context *fc, struct fs_parameter *param)
>  {
> -	substring_t args[MAX_OPT_ARGS];
> -	int option;
> -	int token;
> +	struct tracefs_fs_info *opts = fc->s_fs_info;
> +	struct fs_parse_result result;
>  	kuid_t uid;
>  	kgid_t gid;
> -	char *p;
> -
> -	opts->mode = TRACEFS_DEFAULT_MODE;
> -
> -	while ((p = strsep(&data, ",")) != NULL) {
> -		if (!*p)
> -			continue;
> -
> -		token = match_token(p, tokens, args);
> -		switch (token) {
> -		case Opt_uid:
> -			if (match_int(&args[0], &option))
> -				return -EINVAL;
> -			uid = make_kuid(current_user_ns(), option);
> -			if (!uid_valid(uid))
> -				return -EINVAL;
> -			opts->uid = uid;
> -			break;
> -		case Opt_gid:
> -			if (match_int(&args[0], &option))
> -				return -EINVAL;
> -			gid = make_kgid(current_user_ns(), option);
> -			if (!gid_valid(gid))
> -				return -EINVAL;
> -			opts->gid = gid;
> -			break;
> -		case Opt_mode:
> -			if (match_octal(&args[0], &option))
> -				return -EINVAL;
> -			opts->mode = option & S_IALLUGO;
> -			break;
> +	int opt;
> +
> +	opt = fs_parse(fc, &tracefs_fs_parameters, param, &result);
> +	if (opt < 0)
> +		return opt;
> +
> +	switch (opt) {
> +	case Opt_uid:
> +		uid = make_kuid(current_user_ns(), result.uint_32);
> +		if (!uid_valid(uid))
> +			return invalf(fc, "Unknown uid");
> +		opts->uid = uid;
> +		break;
> +	case Opt_gid:
> +		gid = make_kgid(current_user_ns(), result.uint_32);
> +		if (!gid_valid(gid))
> +			return invalf(fc, "Unknown gid");
> +		opts->gid = gid;
> +		break;
> +	case Opt_mode:
> +		opts->mode = result.uint_32 & S_IALLUGO;
> +		break;
>  		/*
>  		 * We might like to report bad mount options here;
>  		 * but traditionally tracefs has ignored all mount options
>  		 */
> -		}
>  	}
>  
>  	return 0;
> @@ -214,100 +204,96 @@ static int tracefs_apply_options(struct super_block *sb)
>  {
>  	struct tracefs_fs_info *fsi = sb->s_fs_info;
>  	struct inode *inode = sb->s_root->d_inode;
> -	struct tracefs_mount_opts *opts = &fsi->mount_opts;
>  
>  	inode->i_mode &= ~S_IALLUGO;
> -	inode->i_mode |= opts->mode;
> +	inode->i_mode |= fsi->mode;
>  
> -	inode->i_uid = opts->uid;
> -	inode->i_gid = opts->gid;
> +	inode->i_uid = fsi->uid;
> +	inode->i_gid = fsi->gid;
>  
>  	return 0;
>  }
>  
> -static int tracefs_remount(struct super_block *sb, int *flags, char *data)
> +static int tracefs_reconfigure(struct fs_context *fc)
>  {
> -	int err;
> -	struct tracefs_fs_info *fsi = sb->s_fs_info;
> +	struct super_block *sb = fc->root->d_sb;
>  
>  	sync_filesystem(sb);
> -	err = tracefs_parse_options(data, &fsi->mount_opts);
> -	if (err)
> -		goto fail;
> -
> -	tracefs_apply_options(sb);
> -
> -fail:
> -	return err;
> +	return tracefs_apply_options(sb);
>  }
>  
>  static int tracefs_show_options(struct seq_file *m, struct dentry *root)
>  {
>  	struct tracefs_fs_info *fsi = root->d_sb->s_fs_info;
> -	struct tracefs_mount_opts *opts = &fsi->mount_opts;
>  
> -	if (!uid_eq(opts->uid, GLOBAL_ROOT_UID))
> +	if (!uid_eq(fsi->uid, GLOBAL_ROOT_UID))
>  		seq_printf(m, ",uid=%u",
> -			   from_kuid_munged(&init_user_ns, opts->uid));
> -	if (!gid_eq(opts->gid, GLOBAL_ROOT_GID))
> +			   from_kuid_munged(&init_user_ns, fsi->uid));
> +	if (!gid_eq(fsi->gid, GLOBAL_ROOT_GID))
>  		seq_printf(m, ",gid=%u",
> -			   from_kgid_munged(&init_user_ns, opts->gid));
> -	if (opts->mode != TRACEFS_DEFAULT_MODE)
> -		seq_printf(m, ",mode=%o", opts->mode);
> +			   from_kgid_munged(&init_user_ns, fsi->gid));
> +	if (fsi->mode != TRACEFS_DEFAULT_MODE)
> +		seq_printf(m, ",mode=%o", fsi->mode);
>  
>  	return 0;
>  }
>  
>  static const struct super_operations tracefs_super_operations = {
>  	.statfs		= simple_statfs,
> -	.remount_fs	= tracefs_remount,
>  	.show_options	= tracefs_show_options,
>  };
>  
> -static int trace_fill_super(struct super_block *sb, void *data, int silent)
> +static int tracefs_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>  	static const struct tree_descr trace_files[] = {{""}};
> -	struct tracefs_fs_info *fsi;
>  	int err;
>  
> -	fsi = kzalloc(sizeof(struct tracefs_fs_info), GFP_KERNEL);
> -	sb->s_fs_info = fsi;
> -	if (!fsi) {
> -		err = -ENOMEM;
> -		goto fail;
> -	}
> -
> -	err = tracefs_parse_options(data, &fsi->mount_opts);
> +	err = simple_fill_super(sb, TRACEFS_MAGIC, trace_files);
>  	if (err)
> -		goto fail;
> -
> -	err  =  simple_fill_super(sb, TRACEFS_MAGIC, trace_files);
> -	if (err)
> -		goto fail;
> +		return err;
>  
>  	sb->s_op = &tracefs_super_operations;
> +	return tracefs_apply_options(sb);
> +}
>  
> -	tracefs_apply_options(sb);
> -
> -	return 0;
> +static int tracefs_get_tree(struct fs_context *fc)
> +{
> +	return vfs_get_super(fc, vfs_get_single_reconf_super,
> +			     tracefs_fill_super);
> +}
>  
> -fail:
> -	kfree(fsi);
> -	sb->s_fs_info = NULL;
> -	return err;
> +static void tracefs_free_fc(struct fs_context *fc)
> +{
> +	kfree(fc->s_fs_info);
>  }
>  
> -static struct dentry *trace_mount(struct file_system_type *fs_type,
> -			int flags, const char *dev_name,
> -			void *data)
> +static const struct fs_context_operations tracefs_context_ops = {
> +	.free		= tracefs_free_fc,
> +	.parse_param	= tracefs_parse_param,
> +	.get_tree	= tracefs_get_tree,
> +	.reconfigure	= tracefs_reconfigure,
> +};
> +
> +static int tracefs_init_fs_context(struct fs_context *fc)
>  {
> -	return mount_single(fs_type, flags, data, trace_fill_super);
> +	struct tracefs_fs_info *fsi;
> +
> +	fsi = kzalloc(sizeof(struct tracefs_fs_info), GFP_KERNEL);
> +	if (!fsi)
> +		return -ENOMEM;
> +
> +	fsi->mode = TRACEFS_DEFAULT_MODE;
> +
> +	fc->s_fs_info = fsi;
> +	fc->ops = &tracefs_context_ops;
> +	return 0;
>  }
>  
>  static struct file_system_type trace_fs_type = {
>  	.owner =	THIS_MODULE,
>  	.name =		"tracefs",
> -	.mount =	trace_mount,
> +	.init_fs_context = tracefs_init_fs_context,
> +	.parameters	= &tracefs_fs_parameters,
>  	.kill_sb =	kill_litter_super,
>  };
>  MODULE_ALIAS_FS("tracefs");


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

* Re: [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
  2019-03-21 12:58   ` Steven Rostedt
@ 2019-03-21 15:49   ` David Howells
  2019-03-21 15:59     ` Steven Rostedt
  2019-03-21 17:13     ` David Howells
  2019-03-21 15:50   ` David Howells
  2 siblings, 2 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 15:49 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: dhowells, viro, Greg Kroah-Hartman, linux-fsdevel, linux-kernel

How about this:

"""
Convert tracefs to the new internal mount API as the old one will be
obsoleted and removed.  This allows greater flexibility in communication of
mount parameters between userspace, the VFS and the filesystem.

See Documentation/filesystems/mount_api.txt
"""

I should update the subjects to say "Convert xxxfs to use the new mount
API" too.

David

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

* Re: [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
  2019-03-21 12:58   ` Steven Rostedt
  2019-03-21 15:49   ` David Howells
@ 2019-03-21 15:50   ` David Howells
  2 siblings, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 15:50 UTC (permalink / raw)
  Cc: dhowells, Steven Rostedt, viro, Greg Kroah-Hartman,
	linux-fsdevel, linux-kernel

David Howells <dhowells@redhat.com> wrote:

> See Documentation/filesystems/mount_api.txt

And that document is slightly out of date, so I'll need to post an update for
that too.

David

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

* Re: [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 15:49   ` David Howells
@ 2019-03-21 15:59     ` Steven Rostedt
  2019-03-21 17:13     ` David Howells
  1 sibling, 0 replies; 17+ messages in thread
From: Steven Rostedt @ 2019-03-21 15:59 UTC (permalink / raw)
  To: David Howells; +Cc: viro, Greg Kroah-Hartman, linux-fsdevel, linux-kernel

On Thu, 21 Mar 2019 15:49:00 +0000
David Howells <dhowells@redhat.com> wrote:

> How about this:
> 
> """
> Convert tracefs to the new internal mount API as the old one will be
> obsoleted and removed.  This allows greater flexibility in communication of
> mount parameters between userspace, the VFS and the filesystem.
> 
> See Documentation/filesystems/mount_api.txt
> """

Better than none ;-)

> 
> I should update the subjects to say "Convert xxxfs to use the new mount
> API" too.
> 

Yep.

-- Steve

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

* Re: [RFC PATCH 4/8] vfs: Convert pstore to fs_context
  2019-03-21 11:48 ` [RFC PATCH 4/8] vfs: Convert pstore " David Howells
@ 2019-03-21 16:31   ` Kees Cook
  2019-03-21 17:03   ` David Howells
  1 sibling, 0 replies; 17+ messages in thread
From: Kees Cook @ 2019-03-21 16:31 UTC (permalink / raw)
  To: David Howells
  Cc: Al Viro, Anton Vorontsov, Colin Cross, Tony Luck, linux-fsdevel, LKML

On Thu, Mar 21, 2019 at 4:48 AM David Howells <dhowells@redhat.com> wrote:
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Kees Cook <keescook@chromium.org>
> cc: Anton Vorontsov <anton@enomsg.org>
> cc: Colin Cross <ccross@android.com>
> cc: Tony Luck <tony.luck@intel.com>

Thanks for doing this conversion!

Acked-by: Kees Cook <keescook@chromium.org>

Some questions/nits below:

> +static int pstore_parse_param(struct fs_context *fc, struct fs_parameter *param)
> +{
> +       struct pstore_fs_context *ctx = fc->fs_private;
> +       struct fs_parse_result result;
> +       int opt;
> +
> +       opt = fs_parse(fc, &pstore_fs_parameters, param, &result);
> +       if (opt < 0)
> +               return opt;
> +
> +       switch (opt) {
> +       case Opt_kmsg_bytes:
> +               ctx->kmsg_bytes = result.uint_32;
> +               break;
> +       }
>
> -       while ((p = strsep(&options, ",")) != NULL) {
> -               int token;
> +       return 0;
> +}
>
> -               if (!*p)
> -                       continue;
> +static void pstore_apply_param(struct fs_context *fc)
> +{
> +       struct pstore_fs_context *ctx = fc->fs_private;
>
> -               token = match_token(p, tokens, args);
> -               switch (token) {
> -               case Opt_kmsg_bytes:
> -                       if (!match_int(&args[0], &option))
> -                               pstore_set_kmsg_bytes(option);
> -                       break;
> -               }
> -       }
> +       pstore_set_kmsg_bytes(ctx->kmsg_bytes);
>  }

Why the separation between parse and apply now? Is this due to the
reconfigure calls? (i.e. why not call pstore_set_kmsg_bytes() in
pstore_parse_param()?

> @@ -416,14 +423,38 @@ static int pstore_fill_super(struct super_block *sb, void *data, int silent)
>                 return -ENOMEM;
>
>         pstore_get_records(0);
> -
>         return 0;
>  }

I prefer to keep a blank before "return", but no need to drop this
unless it gets a respin.

Thanks again!

-- 
Kees Cook

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

* Re: [RFC PATCH 4/8] vfs: Convert pstore to fs_context
  2019-03-21 11:48 ` [RFC PATCH 4/8] vfs: Convert pstore " David Howells
  2019-03-21 16:31   ` Kees Cook
@ 2019-03-21 17:03   ` David Howells
  2019-03-21 17:35     ` Kees Cook
  1 sibling, 1 reply; 17+ messages in thread
From: David Howells @ 2019-03-21 17:03 UTC (permalink / raw)
  To: Kees Cook
  Cc: dhowells, Al Viro, Anton Vorontsov, Colin Cross, Tony Luck,
	linux-fsdevel, LKML

Kees Cook <keescook@chromium.org> wrote:

> Why the separation between parse and apply now? Is this due to the
> reconfigure calls? (i.e. why not call pstore_set_kmsg_bytes() in
> pstore_parse_param()?

Because parameter parsing is now done up front, before the creation of the
superblock of the invocation of the reconfigure method - so there's still a
bunch of places that can error out before you know you're going to be
successful in creating/reconfiguring the superblock.

David

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

* Re: [RFC PATCH 3/8] vfs: Convert tracefs to fs_context
  2019-03-21 15:49   ` David Howells
  2019-03-21 15:59     ` Steven Rostedt
@ 2019-03-21 17:13     ` David Howells
  1 sibling, 0 replies; 17+ messages in thread
From: David Howells @ 2019-03-21 17:13 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: dhowells, viro, Greg Kroah-Hartman, linux-fsdevel, linux-kernel

Steven Rostedt <rostedt@goodmis.org> wrote:

> > See Documentation/filesystems/mount_api.txt

Here's the revised document for you to look over.

David
---
			     ====================
			     FILESYSTEM MOUNT API
			     ====================

CONTENTS

 (1) Overview.

 (2) The filesystem context.

 (3) The filesystem context operations.

 (4) Filesystem context security.

 (5) VFS filesystem context API.

 (6) Superblock creation helpers.

 (7) Parameter description.

 (8) Parameter helper functions.


========
OVERVIEW
========

The creation of new mounts is now to be done in a multistep process:

 (1) Create a filesystem context.

 (2) Parse the parameters and attach them to the context.  Parameters are
     expected to be passed individually from userspace, though legacy binary
     parameters can also be handled.

 (3) Validate and pre-process the context.

 (4) Get or create a superblock and mountable root.

 (5) Perform the mount.

 (6) Return an error message attached to the context.

 (7) Destroy the context.

To support this, the file_system_type struct gains two new fields:

	int (*init_fs_context)(struct fs_context *fc);
	const struct fs_parameter_description *parameters;

The first is invoked to set up the filesystem-specific parts of a filesystem
context, including the additional space, and the second points to the
parameter description for validation at registration time and querying by a
future system call.

Note that security initialisation is done *after* the filesystem is called so
that the namespaces may be adjusted first.


======================
THE FILESYSTEM CONTEXT
======================

The creation and reconfiguration of a superblock is governed by a filesystem
context.  This is represented by the fs_context structure:

	struct fs_context {
		const struct fs_context_operations *ops;
		struct file_system_type *fs_type;
		void			*fs_private;
		struct dentry		*root;
		struct user_namespace	*user_ns;
		struct net		*net_ns;
		const struct cred	*cred;
		char			*source;
		char			*subtype;
		void			*security;
		void			*s_fs_info;
		unsigned int		sb_flags;
		unsigned int		sb_flags_mask;
		unsigned int		s_iflags;
		unsigned int		lsm_flags;
		enum fs_context_purpose	purpose:8;
		...
	};

The fs_context fields are as follows:

 (*) const struct fs_context_operations *ops

     These are operations that can be done on a filesystem context (see
     below).  This must be set by the ->init_fs_context() file_system_type
     operation.

 (*) struct file_system_type *fs_type

     A pointer to the file_system_type of the filesystem that is being
     constructed or reconfigured.  This retains a reference on the type owner.

 (*) void *fs_private

     A pointer to the file system's private data.  This is where the filesystem
     will need to store any options it parses.

 (*) struct dentry *root

     A pointer to the root of the mountable tree (and indirectly, the
     superblock thereof).  This is filled in by the ->get_tree() op.  If this
     is set, an active reference on root->d_sb must also be held.

 (*) struct user_namespace *user_ns
 (*) struct net *net_ns

     There are a subset of the namespaces in use by the invoking process.  They
     retain references on each namespace.  The subscribed namespaces may be
     replaced by the filesystem to reflect other sources, such as the parent
     mount superblock on an automount.

 (*) const struct cred *cred

     The mounter's credentials.  This retains a reference on the credentials.

 (*) char *source

     This specifies the source.  It may be a block device (e.g. /dev/sda1) or
     something more exotic, such as the "host:/path" that NFS desires.

 (*) char *subtype

     This is a string to be added to the type displayed in /proc/mounts to
     qualify it (used by FUSE).  This is available for the filesystem to set if
     desired.

 (*) void *security

     A place for the LSMs to hang their security data for the superblock.  The
     relevant security operations are described below.

 (*) void *s_fs_info

     The proposed s_fs_info for a new superblock, set in the superblock by
     sget_fc().  This can be used to distinguish superblocks.

 (*) unsigned int sb_flags
 (*) unsigned int sb_flags_mask

     Which bits SB_* flags are to be set/cleared in super_block::s_flags.

 (*) unsigned int s_iflags

     These will be bitwise-OR'd with s->s_iflags when a superblock is created.

 (*) enum fs_context_purpose

     This indicates the purpose for which the context is intended.  The
     available values are:

	FS_CONTEXT_FOR_MOUNT,		-- New superblock for explicit mount
	FS_CONTEXT_FOR_SUBMOUNT		-- New automatic submount of extant mount
	FS_CONTEXT_FOR_RECONFIGURE	-- Change an existing mount

The mount context is created by calling vfs_new_fs_context() or
vfs_dup_fs_context() and is destroyed with put_fs_context().  Note that the
structure is not refcounted.

VFS, security and filesystem mount options are set individually with
vfs_parse_mount_option().  Options provided by the old mount(2) system call as
a page of data can be parsed with generic_parse_monolithic().

When mounting, the filesystem is allowed to take data from any of the pointers
and attach it to the superblock (or whatever), provided it clears the pointer
in the mount context.

The filesystem is also allowed to allocate resources and pin them with the
mount context.  For instance, NFS might pin the appropriate protocol version
module.


=================================
THE FILESYSTEM CONTEXT OPERATIONS
=================================

The filesystem context points to a table of operations:

	struct fs_context_operations {
		void (*free)(struct fs_context *fc);
		int (*dup)(struct fs_context *fc, struct fs_context *src_fc);
		int (*parse_param)(struct fs_context *fc,
				   struct struct fs_parameter *param);
		int (*parse_monolithic)(struct fs_context *fc, void *data);
		int (*get_tree)(struct fs_context *fc);
		int (*reconfigure)(struct fs_context *fc);
	};

These operations are invoked by the various stages of the mount procedure to
manage the filesystem context.  They are as follows:

 (*) void (*free)(struct fs_context *fc);

     Called to clean up the filesystem-specific part of the filesystem context
     when the context is destroyed.  It should be aware that parts of the
     context may have been removed and NULL'd out by ->get_tree().

 (*) int (*dup)(struct fs_context *fc, struct fs_context *src_fc);

     Called when a filesystem context has been duplicated to duplicate the
     filesystem-private data.  An error may be returned to indicate failure to
     do this.

     [!] Note that even if this fails, put_fs_context() will be called
	 immediately thereafter, so ->dup() *must* make the
	 filesystem-private data safe for ->free().

 (*) int (*parse_param)(struct fs_context *fc,
			struct struct fs_parameter *param);

     Called when a parameter is being added to the filesystem context.  param
     points to the key name and maybe a value object.  VFS-specific options
     will have been weeded out and fc->sb_flags updated in the context.
     Security options will also have been weeded out and fc->security updated.

     The parameter can be parsed with fs_parse() and fs_lookup_param().  Note
     that the source(s) are presented as parameters named "source".

     If successful, 0 should be returned or a negative error code otherwise.

 (*) int (*parse_monolithic)(struct fs_context *fc, void *data);

     Called when the mount(2) system call is invoked to pass the entire data
     page in one go.  If this is expected to be just a list of "key[=val]"
     items separated by commas, then this may be set to NULL.

     The return value is as for ->parse_param().

     If the filesystem (e.g. NFS) needs to examine the data first and then
     finds it's the standard key-val list then it may pass it off to
     generic_parse_monolithic().

 (*) int (*get_tree)(struct fs_context *fc);

     Called to get or create the mountable root and superblock, using the
     information stored in the filesystem context (reconfiguration goes via a
     different vector).  It may detach any resources it desires from the
     filesystem context and transfer them to the superblock it creates.

     On success it should set fc->root to the mountable root and return 0.  In
     the case of an error, it should return a negative error code.

     The phase on a userspace-driven context will be set to only allow this to
     be called once on any particular context.

 (*) int (*reconfigure)(struct fs_context *fc);

     Called to effect reconfiguration of a superblock using information stored
     in the filesystem context.  It may detach any resources it desires from
     the filesystem context and transfer them to the superblock.  The
     superblock can be found from fc->root->d_sb.

     On success it should return 0.  In the case of an error, it should return
     a negative error code.

     [NOTE] reconfigure is intended as a replacement for remount_fs.


===========================
FILESYSTEM CONTEXT SECURITY
===========================

The filesystem context contains a security pointer that the LSMs can use for
building up a security context for the superblock to be mounted.  There are a
number of operations used by the new mount code for this purpose:

 (*) int security_fs_context_alloc(struct fs_context *fc,
				   struct dentry *reference);

     Called to initialise fc->security (which is preset to NULL) and allocate
     any resources needed.  It should return 0 on success or a negative error
     code on failure.

     reference will be non-NULL if the context is being created for superblock
     reconfiguration (FS_CONTEXT_FOR_RECONFIGURE) in which case it indicates
     the root dentry of the superblock to be reconfigured.  It will also be
     non-NULL in the case of a submount (FS_CONTEXT_FOR_SUBMOUNT) in which case
     it indicates the automount point.

 (*) int security_fs_context_dup(struct fs_context *fc,
				 struct fs_context *src_fc);

     Called to initialise fc->security (which is preset to NULL) and allocate
     any resources needed.  The original filesystem context is pointed to by
     src_fc and may be used for reference.  It should return 0 on success or a
     negative error code on failure.

 (*) void security_fs_context_free(struct fs_context *fc);

     Called to clean up anything attached to fc->security.  Note that the
     contents may have been transferred to a superblock and the pointer cleared
     during get_tree.

 (*) int security_fs_context_parse_param(struct fs_context *fc,
					 struct fs_parameter *param);

     Called for each mount parameter, including the source.  The arguments are
     as for the ->parse_param() method.  It should return 0 to indicate that
     the parameter should be passed on to the filesystem, 1 to indicate that
     the parameter should be discarded or an error to indicate that the
     parameter should be rejected.

     The value pointed to by param may be modified (if a string) or stolen
     (provided the value pointer is NULL'd out).  If it is stolen, 1 must be
     returned to prevent it being passed to the filesystem.

 (*) int security_fs_context_validate(struct fs_context *fc);

     Called after all the options have been parsed to validate the collection
     as a whole and to do any necessary allocation so that
     security_sb_get_tree() and security_sb_reconfigure() are less likely to
     fail.  It should return 0 or a negative error code.

     In the case of reconfiguration, the target superblock will be accessible
     via fc->root.

 (*) int security_sb_get_tree(struct fs_context *fc);

     Called during the mount procedure to verify that the specified superblock
     is allowed to be mounted and to transfer the security data there.  It
     should return 0 or a negative error code.

 (*) void security_sb_reconfigure(struct fs_context *fc);

     Called to apply any reconfiguration to an LSM's context.  It must not
     fail.  Error checking and resource allocation must be done in advance by
     the parameter parsing and validation hooks.

 (*) int security_sb_mountpoint(struct fs_context *fc, struct path *mountpoint,
				unsigned int mnt_flags);

     Called during the mount procedure to verify that the root dentry attached
     to the context is permitted to be attached to the specified mountpoint.
     It should return 0 on success or a negative error code on failure.


==========================
VFS FILESYSTEM CONTEXT API
==========================

There are four operations for creating a filesystem context and one for
destroying a context:

 (*) struct fs_context *fs_context_for_mount(
		struct file_system_type *fs_type,
		unsigned int sb_flags);

     Allocate a filesystem context for the purpose of setting up a new mount,
     whether that be with a new superblock or sharing an existing one.  This
     sets the superblock flags, initialises the security and calls
     fs_type->init_fs_context() to initialise the filesystem private data.

     fs_type specifies the filesystem type that will manage the context and
     sb_flags presets the superblock flags stored therein.

 (*) struct fs_context *fs_context_for_reconfigure(
		struct dentry *dentry,
		unsigned int sb_flags,
		unsigned int sb_flags_mask);

     Allocate a filesystem context for the purpose of reconfiguring an
     existing superblock.  dentry provides a reference to the superblock to be
     configured.  sb_flags and sb_flags_mask indicate which superblock flags
     need changing and to what.

 (*) struct fs_context *fs_context_for_submount(
		struct file_system_type *fs_type,
		struct dentry *reference);

     Allocate a filesystem context for the purpose of creating a new mount for
     an automount point or other derived superblock.  fs_type specifies the
     filesystem type that will manage the context and the reference dentry
     supplies the parameters.  Namespaces are propagated from the reference
     dentry's superblock also.

     Note that it's not a requirement that the reference dentry be of the same
     filesystem type as fs_type.

 (*) struct fs_context *vfs_dup_fs_context(struct fs_context *src_fc);

     Duplicate a filesystem context, copying any options noted and duplicating
     or additionally referencing any resources held therein.  This is available
     for use where a filesystem has to get a mount within a mount, such as NFS4
     does by internally mounting the root of the target server and then doing a
     private pathwalk to the target directory.

     The purpose in the new context is inherited from the old one.

 (*) void put_fs_context(struct fs_context *fc);

     Destroy a filesystem context, releasing any resources it holds.  This
     calls the ->free() operation.  This is intended to be called by anyone who
     created a filesystem context.

     [!] filesystem contexts are not refcounted, so this causes unconditional
	 destruction.

In all the above operations, apart from the put op, the return is a mount
context pointer or a negative error code.

For the remaining operations, if an error occurs, a negative error code will be
returned.

 (*) int vfs_parse_fs_param(struct fs_context *fc,
			    struct fs_parameter *param);

     Supply a single mount parameter to the filesystem context.  This include
     the specification of the source/device which is specified as the "source"
     parameter (which may be specified multiple times if the filesystem
     supports that).

     param specifies the parameter key name and the value.  The parameter is
     first checked to see if it corresponds to a standard mount flag (in which
     case it is used to set an SB_xxx flag and consumed) or a security option
     (in which case the LSM consumes it) before it is passed on to the
     filesystem.

     The parameter value is typed and can be one of:

	fs_value_is_flag,		Parameter not given a value.
	fs_value_is_string,		Value is a string
	fs_value_is_blob,		Value is a binary blob
	fs_value_is_filename,		Value is a filename* + dirfd
	fs_value_is_filename_empty,	Value is a filename* + dirfd + AT_EMPTY_PATH
	fs_value_is_file,		Value is an open file (file*)

     If there is a value, that value is stored in a union in the struct in one
     of param->{string,blob,name,file}.  Note that the function may steal and
     clear the pointer, but then becomes responsible for disposing of the
     object.

 (*) int vfs_parse_fs_string(struct fs_context *fc, const char *key,
			     const char *value, size_t v_size);

     A wrapper around vfs_parse_fs_param() that copies the value string it is
     passed.

 (*) int generic_parse_monolithic(struct fs_context *fc, void *data);

     Parse a sys_mount() data page, assuming the form to be a text list
     consisting of key[=val] options separated by commas.  Each item in the
     list is passed to vfs_mount_option().  This is the default when the
     ->parse_monolithic() method is NULL.

 (*) int vfs_get_tree(struct fs_context *fc);

     Get or create the mountable root and superblock, using the parameters in
     the filesystem context to select/configure the superblock.  This invokes
     the ->get_tree() method.

 (*) struct vfsmount *vfs_create_mount(struct fs_context *fc);

     Create a mount given the parameters in the specified filesystem context.
     Note that this does not attach the mount to anything.


===========================
SUPERBLOCK CREATION HELPERS
===========================

A number of VFS helpers are available for use by filesystems for the creation
or looking up of superblocks.

 (*) struct super_block *
     sget_fc(struct fs_context *fc,
	     int (*test)(struct super_block *sb, struct fs_context *fc),
	     int (*set)(struct super_block *sb, struct fs_context *fc));

     This is the core routine.  If test is non-NULL, it searches for an
     existing superblock matching the criteria held in the fs_context, using
     the test function to match them.  If no match is found, a new superblock
     is created and the set function is called to set it up.

     Prior to the set function being called, fc->s_fs_info will be transferred
     to sb->s_fs_info - and fc->s_fs_info will be cleared if set returns
     success (ie. 0).

The following helpers all wrap sget_fc():

 (*) int vfs_get_super(struct fs_context *fc,
		       enum vfs_get_super_keying keying,
		       int (*fill_super)(struct super_block *sb,
					 struct fs_context *fc))

     This creates/looks up a deviceless superblock.  The keying indicates how
     many superblocks of this type may exist and in what manner they may be
     shared:

	(1) vfs_get_single_super

	    Only one such superblock may exist in the system.  Any further
	    attempt to get a new superblock gets this one (and any parameter
	    differences are ignored).

	(2) vfs_get_keyed_super

	    Multiple superblocks of this type may exist and they're keyed on
	    their s_fs_info pointer (for example this may refer to a
	    namespace).

	(3) vfs_get_independent_super

	    Multiple independent superblocks of this type may exist.  This
	    function never matches an existing one and always creates a new
	    one.


=====================
PARAMETER DESCRIPTION
=====================

Parameters are described using structures defined in linux/fs_parser.h.
There's a core description struct that links everything together:

	struct fs_parameter_description {
		const char	name[16];
		const struct fs_parameter_spec *specs;
		const struct fs_parameter_enum *enums;
	};

For example:

	enum {
		Opt_autocell,
		Opt_bar,
		Opt_dyn,
		Opt_foo,
		Opt_source,
	};

	static const struct fs_parameter_description afs_fs_parameters = {
		.name		= "kAFS",
		.specs		= afs_param_specs,
		.enums		= afs_param_enums,
	};

The members are as follows:

 (1) const char name[16];

     The name to be used in error messages generated by the parse helper
     functions.

 (2) const struct fs_parameter_specification *specs;

     Table of parameter specifications, terminated with a null entry, where the
     entries are of type:

	struct fs_parameter_spec {
		const char		*name;
		u8			opt;
		enum fs_parameter_type	type:8;
		unsigned short		flags;
	};

     The 'name' field is a string to match exactly to the parameter key (no
     wildcards, patterns and no case-independence) and 'opt' is the value that
     will be returned by the fs_parser() function in the case of a successful
     match.

     The 'type' field indicates the desired value type and must be one of:

	TYPE NAME		EXPECTED VALUE		RESULT IN
	=======================	=======================	=====================
	fs_param_is_flag	No value		n/a
	fs_param_is_bool	Boolean value		result->boolean
	fs_param_is_u32		32-bit unsigned int	result->uint_32
	fs_param_is_u32_octal	32-bit octal int	result->uint_32
	fs_param_is_u32_hex	32-bit hex int		result->uint_32
	fs_param_is_s32		32-bit signed int	result->int_32
	fs_param_is_u64		64-bit unsigned int	result->uint_64
	fs_param_is_enum	Enum value name 	result->uint_32
	fs_param_is_string	Arbitrary string	param->string
	fs_param_is_blob	Binary blob		param->blob
	fs_param_is_blockdev	Blockdev path		* Needs lookup
	fs_param_is_path	Path			* Needs lookup
	fs_param_is_fd		File descriptor		result->int_32

     Note that if the value is of fs_param_is_bool type, fs_parse() will try
     to match any string value against "0", "1", "no", "yes", "false", "true".

     Each parameter can also be qualified with 'flags':

     	fs_param_v_optional	The value is optional
	fs_param_neg_with_no	result->negated set if key is prefixed with "no"
	fs_param_neg_with_empty	result->negated set if value is ""
	fs_param_deprecated	The parameter is deprecated.

     These are wrapped with a number of convenience wrappers:

	MACRO			SPECIFIES
	=======================	===============================================
	fsparam_flag()		fs_param_is_flag
	fsparam_flag_no()	fs_param_is_flag, fs_param_neg_with_no
	fsparam_bool()		fs_param_is_bool
	fsparam_u32()		fs_param_is_u32
	fsparam_u32oct()	fs_param_is_u32_octal
	fsparam_u32hex()	fs_param_is_u32_hex
	fsparam_s32()		fs_param_is_s32
	fsparam_u64()		fs_param_is_u64
	fsparam_enum()		fs_param_is_enum
	fsparam_string()	fs_param_is_string
	fsparam_blob()		fs_param_is_blob
	fsparam_bdev()		fs_param_is_blockdev
	fsparam_path()		fs_param_is_path
	fsparam_fd()		fs_param_is_fd

     all of which take two arguments, name string and option number - for
     example:

	static const struct fs_parameter_spec afs_param_specs[] = {
		fsparam_flag	("autocell",	Opt_autocell),
		fsparam_flag	("dyn",		Opt_dyn),
		fsparam_string	("source",	Opt_source),
		fsparam_flag_no	("foo",		Opt_foo),
		{}
	};

     An addition macro, __fsparam() is provided that takes an additional pair
     of arguments to specify the type and the flags for anything that doesn't
     match one of the above macros.

 (6) const struct fs_parameter_enum *enums;

     Table of enum value names to integer mappings, terminated with a null
     entry.  This is of type:

	struct fs_parameter_enum {
		u8		opt;
		char		name[14];
		u8		value;
	};

     Where the array is an unsorted list of { parameter ID, name }-keyed
     elements that indicate the value to map to, e.g.:

	static const struct fs_parameter_enum afs_param_enums[] = {
		{ Opt_bar,   "x",      1},
		{ Opt_bar,   "y",      23},
		{ Opt_bar,   "z",      42},
	};

     If a parameter of type fs_param_is_enum is encountered, fs_parse() will
     try to look the value up in the enum table and the result will be stored
     in the parse result.

The parser should be pointed to by the parser pointer in the file_system_type
struct as this will provide validation on registration (if
CONFIG_VALIDATE_FS_PARSER=y) and will allow the description to be queried from
userspace using the fsinfo() syscall.


==========================
PARAMETER HELPER FUNCTIONS
==========================

A number of helper functions are provided to help a filesystem or an LSM
process the parameters it is given.

 (*) int lookup_constant(const struct constant_table tbl[],
			 const char *name, int not_found);

     Look up a constant by name in a table of name -> integer mappings.  The
     table is an array of elements of the following type:

	struct constant_table {
		const char	*name;
		int		value;
	};

     If a match is found, the corresponding value is returned.  If a match
     isn't found, the not_found value is returned instead.

 (*) bool validate_constant_table(const struct constant_table *tbl,
				  size_t tbl_size,
				  int low, int high, int special);

     Validate a constant table.  Checks that all the elements are appropriately
     ordered, that there are no duplicates and that the values are between low
     and high inclusive, though provision is made for one allowable special
     value outside of that range.  If no special value is required, special
     should just be set to lie inside the low-to-high range.

     If all is good, true is returned.  If the table is invalid, errors are
     logged to dmesg and false is returned.

 (*) bool fs_validate_description(const struct fs_parameter_description *desc);

     This performs some validation checks on a parameter description.  It
     returns true if the description is good and false if it is not.  It will
     log errors to dmesg if validation fails.

 (*) int fs_parse(struct fs_context *fc,
		  const struct fs_parameter_description *desc,
		  struct fs_parameter *param,
		  struct fs_parse_result *result);

     This is the main interpreter of parameters.  It uses the parameter
     description to look up a parameter by key name and to convert that to an
     option number (which it returns).

     If successful, and if the parameter type indicates the result is a
     boolean, integer or enum type, the value is converted by this function and
     the result stored in result->{boolean,int_32,uint_32,uint_64}.

     If a match isn't initially made, the key is prefixed with "no" and no
     value is present then an attempt will be made to look up the key with the
     prefix removed.  If this matches a parameter for which the type has flag
     fs_param_neg_with_no set, then a match will be made and result->negated
     will be set to true.

     If the parameter isn't matched, -ENOPARAM will be returned; if the
     parameter is matched, but the value is erroneous, -EINVAL will be
     returned; otherwise the parameter's option number will be returned.

 (*) int fs_lookup_param(struct fs_context *fc,
			 struct fs_parameter *value,
			 bool want_bdev,
			 struct path *_path);

     This takes a parameter that carries a string or filename type and attempts
     to do a path lookup on it.  If the parameter expects a blockdev, a check
     is made that the inode actually represents one.

     Returns 0 if successful and *_path will be set; returns a negative error
     code if not.

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

* Re: [RFC PATCH 4/8] vfs: Convert pstore to fs_context
  2019-03-21 17:03   ` David Howells
@ 2019-03-21 17:35     ` Kees Cook
  0 siblings, 0 replies; 17+ messages in thread
From: Kees Cook @ 2019-03-21 17:35 UTC (permalink / raw)
  To: David Howells
  Cc: Al Viro, Anton Vorontsov, Colin Cross, Tony Luck, linux-fsdevel, LKML

On Thu, Mar 21, 2019 at 10:03 AM David Howells <dhowells@redhat.com> wrote:
>
> Kees Cook <keescook@chromium.org> wrote:
>
> > Why the separation between parse and apply now? Is this due to the
> > reconfigure calls? (i.e. why not call pstore_set_kmsg_bytes() in
> > pstore_parse_param()?
>
> Because parameter parsing is now done up front, before the creation of the
> superblock of the invocation of the reconfigure method - so there's still a
> bunch of places that can error out before you know you're going to be
> successful in creating/reconfiguring the superblock.

Thanks! Makes sense. :)


-- 
Kees Cook

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

end of thread, other threads:[~2019-03-21 17:35 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-21 11:47 [RFC PATCH 0/8] Convert mount_single-using filesystems to fs_context David Howells
2019-03-21 11:47 ` [RFC PATCH 1/8] vfs: Add a single-or-reconfig keying to vfs_get_super() David Howells
2019-03-21 11:47 ` [RFC PATCH 2/8] vfs: Convert debugfs to fs_context David Howells
2019-03-21 11:47 ` [RFC PATCH 3/8] vfs: Convert tracefs " David Howells
2019-03-21 12:58   ` Steven Rostedt
2019-03-21 15:49   ` David Howells
2019-03-21 15:59     ` Steven Rostedt
2019-03-21 17:13     ` David Howells
2019-03-21 15:50   ` David Howells
2019-03-21 11:48 ` [RFC PATCH 4/8] vfs: Convert pstore " David Howells
2019-03-21 16:31   ` Kees Cook
2019-03-21 17:03   ` David Howells
2019-03-21 17:35     ` Kees Cook
2019-03-21 11:48 ` [RFC PATCH 5/8] hypfs: Fix error number left in struct pointer member David Howells
2019-03-21 11:48 ` [RFC PATCH 6/8] vfs: Convert hypfs to fs_context David Howells
2019-03-21 11:48 ` [RFC PATCH 7/8] vfs: Convert spufs " David Howells
2019-03-21 11:48 ` [RFC PATCH 8/8] vfs: Kill off mount_single() David Howells

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