Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1641798 > unrolled thread

[RFC][PATCH 00/21] VFS: Introduce superblock configuration context [ver #3]

Started byDavid Howells <dhowells@redhat.com>
First post2017-05-15 17:20 +0200
Last post2017-05-19 16:10 +0200
Articles 18 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC][PATCH 00/21] VFS: Introduce superblock configuration context  [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:20 +0200
    [PATCH 08/21] Implement fsmount() to effect a pre-configured mount  [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:20 +0200
    [PATCH 02/21] Clean up whitespace in fs/namespace.c [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:20 +0200
    [PATCH 01/21] Provide a function to create a NUL-terminated string  from unterminated data [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:20 +0200
    [PATCH 05/21] VFS: Provide empty name qstr [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:20 +0200
    [PATCH 07/21] Implement fsopen() to prepare for a mount [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 18/21] NFS: Do some tidying of the parsing code [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 17/21] NFS: Add a small buffer in nfs_sb_config to avoid  string dup [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 11/21] proc: Add superblock config support to procfs [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 15/21] NFS: Split nfs_parse_mount_options() [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 20/21] Support legacy filesystems [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 13/21] NFS: Constify mount argument match tables [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    [PATCH 16/21] NFS: Deindent nfs_sb_config_parse_option() [ver #3] David Howells <dhowells@redhat.com> - 2017-05-15 17:30 +0200
    Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3] David Howells <dhowells@redhat.com> - 2017-05-16 18:40 +0200
      Re: [PATCH 06/21] VFS: Introduce a superblock configuration context  [ver #3] Miklos Szeredi <mszeredi@redhat.com> - 2017-05-17 10:00 +0200
        Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3] David Howells <dhowells@redhat.com> - 2017-05-17 13:40 +0200
          Re: [PATCH 06/21] VFS: Introduce a superblock configuration context  [ver #3] Miklos Szeredi <mszeredi@redhat.com> - 2017-05-18 10:20 +0200
            Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3] David Howells <dhowells@redhat.com> - 2017-05-19 16:10 +0200

#1641798 — [RFC][PATCH 00/21] VFS: Introduce superblock configuration context [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:20 +0200
Subject[RFC][PATCH 00/21] VFS: Introduce superblock configuration context [ver #3]
Message-ID<tHpWV-1pB-9@gated-at.bofh.it>
Here are a set of patches to create a superblock configuration context
prior to setting up a new mount, populating it with the parsed
options/binary data, creating the superblock and then effecting the mount.

This allows namespaces and other information to be conveyed through the
mount procedure.  It also allows extra error information to be returned
(so many things can go wrong during a mount that a small integer isn't
really sufficient to convey the issue).

This also allows Miklós Szeredi's idea of doing:

	fd = fsopen("nfs");
	write(fd, "option=val", ...);
	fsmount(fd, "/mnt");

that he presented at LSF-2017 to be implemented (see the relevant patches
in the series), to which I can add:

	read(fd, error_buffer, ...);

to read back any error message.  I didn't use netlink as that would make it
depend on CONFIG_NET and would introduce network namespacing issues.

I've implemented mount context handling for procfs and nfs.

Significant changes:

 ver #3:

 (*) Rebased on 4.12-rc1.

 (*) Split the NFS patch up somewhat.

 ver #2:

 (*) Removed the ->fill_super() from sb_config_operations and passed it in
     directly to functions that want to call it.  NFS now calls
     nfs_fill_super() directly rather than jumping through a pointer to it
     since there's only the one option at the moment.

 (*) Removed ->mnt_ns and ->sb from sb_config and moved ->pid_ns into
     proc_sb_config.

 (*) Renamed create_super -> get_tree.

 (*) Renamed struct mount_context to struct sb_config and amended various
     variable names.

 (*) sys_fsmount() acquired AT_* flags and MS_* flags (for MNT_* flags)
     arguments.

 ver #1:

 (*) Split the sb_config stuff out into its own header.

 (*) Support non-context aware filesystems through a special set of
     sb_config operations.

 (*) Stored the created superblock and root dentry into the sb_config after
     creation rather than directly into a vfsmount.  This allows some
     arguments to be removed to various NFS functions.

 (*) Added an explicit superblock-creation step.  This allows a created
     superblock to then be mounted multiple times.

 (*) Added a flag to say that the sb_config is degraded and cannot have
     another go at having a superblock creation whilst getting rid of the
     one that says it's already mounted.

Further developments:

 (*) Implement sb reconfiguration (for now it returns ENOANO).

 (*) Implement mount context support in more filesystems, ext4 being next
     on my list.

 (*) Move the walk-from-root stuff that nfs has to generic code so that you
     can do something akin to:

	mount /dev/sda1:/foo/bar /mnt

     See nfs_follow_remote_path() and mount_subtree().  This is slightly
     tricky in NFS as we have to prevent referral loops.

 (*) Work out how to get at the error message incurred by submounts
     encountered during nfs_follow_remote_path().

     Should the error message be moved to task_struct and made more
     general, perhaps retrieved with a prctl() function?

 (*) Clean up/consolidate the security functions.  Possibly add a
     validation hook to be called at the same time as the mount context
     validate op.

The patches can be found here also:

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

David
---
David Howells (21):
      Provide a function to create a NUL-terminated string from unterminated data
      Clean up whitespace in fs/namespace.c
      VFS: Make get_mnt_ns() return the namespace
      VFS: Make get_filesystem() return the affected filesystem
      VFS: Provide empty name qstr
      VFS: Introduce a superblock configuration context
      Implement fsopen() to prepare for a mount
      Implement fsmount() to effect a pre-configured mount
      Sample program for driving fsopen/fsmount
      procfs: Move proc_fill_super() to fs/proc/root.c
      proc: Add superblock config support to procfs
      NFS: Move mount bits into their own file
      NFS: Constify mount argument match tables
      NFS: Rename struct nfs_parsed_mount_data to struct nfs_sb_config
      NFS: Split nfs_parse_mount_options()
      NFS: Deindent nfs_sb_config_parse_option()
      NFS: Add a small buffer in nfs_sb_config to avoid string dup
      NFS: Do some tidying of the parsing code
      NFS: Add mount context support.
      Support legacy filesystems
      Add commands to create or update a superblock


 Documentation/filesystems/mounting.txt |  470 ++++++++
 arch/x86/entry/syscalls/syscall_32.tbl |    2 
 arch/x86/entry/syscalls/syscall_64.tbl |    2 
 fs/Makefile                            |    3 
 fs/dcache.c                            |    8 
 fs/filesystems.c                       |    3 
 fs/fsopen.c                            |  302 +++++
 fs/gfs2/dir.c                          |    3 
 fs/internal.h                          |    4 
 fs/libfs.c                             |   17 
 fs/mount.h                             |    3 
 fs/namei.c                             |    3 
 fs/namespace.c                         |  495 +++++++--
 fs/nfs/Makefile                        |    2 
 fs/nfs/client.c                        |   74 +
 fs/nfs/getroot.c                       |   76 +
 fs/nfs/internal.h                      |  141 +--
 fs/nfs/mount.c                         | 1499 +++++++++++++++++++++++++++
 fs/nfs/namespace.c                     |   66 +
 fs/nfs/nfs3_fs.h                       |    2 
 fs/nfs/nfs3client.c                    |    6 
 fs/nfs/nfs3proc.c                      |    2 
 fs/nfs/nfs4_fs.h                       |    4 
 fs/nfs/nfs4client.c                    |   82 +
 fs/nfs/nfs4namespace.c                 |  208 ++--
 fs/nfs/nfs4proc.c                      |    3 
 fs/nfs/nfs4super.c                     |  220 ++--
 fs/nfs/proc.c                          |    2 
 fs/nfs/super.c                         | 1785 ++------------------------------
 fs/nsfs.c                              |    3 
 fs/pipe.c                              |    3 
 fs/proc/inode.c                        |   50 -
 fs/proc/internal.h                     |    6 
 fs/proc/root.c                         |  210 +++-
 fs/sb_config.c                         |  524 +++++++++
 fs/super.c                             |  110 +-
 include/linux/dcache.h                 |    5 
 include/linux/fs.h                     |   16 
 include/linux/lsm_hooks.h              |   47 +
 include/linux/mount.h                  |    4 
 include/linux/nfs_xdr.h                |    7 
 include/linux/sb_config.h              |  100 ++
 include/linux/security.h               |   40 +
 include/linux/string.h                 |    1 
 include/linux/syscalls.h               |    3 
 include/uapi/linux/magic.h             |    1 
 kernel/sys_ni.c                        |    4 
 mm/util.c                              |   24 
 samples/fsmount/test-fsmount.c         |   79 +
 security/security.c                    |   35 +
 security/selinux/hooks.c               |  202 +++-
 51 files changed, 4588 insertions(+), 2373 deletions(-)
 create mode 100644 Documentation/filesystems/mounting.txt
 create mode 100644 fs/fsopen.c
 create mode 100644 fs/nfs/mount.c
 create mode 100644 fs/sb_config.c
 create mode 100644 include/linux/sb_config.h
 create mode 100644 samples/fsmount/test-fsmount.c

[toc] | [next] | [standalone]


#1641801 — [PATCH 08/21] Implement fsmount() to effect a pre-configured mount [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:20 +0200
Subject[PATCH 08/21] Implement fsmount() to effect a pre-configured mount [ver #3]
Message-ID<tHpWW-1pB-23@gated-at.bofh.it>
In reply to#1641798
Provide a system call by which a filesystem opened with fsopen() and
configured by a series of writes can be mounted:

	int ret = fsmount(int fsfd, int dfd, const char *path,
			  unsigned int at_flags, unsigned int flags);

where fsfd is the fd returned by fsopen(), dfd, path and at_flags locate
the mountpoint and flags are the applicable MS_* flags.  dfd can be
AT_FDCWD or an fd open to a directory.

In the event that fsmount() fails, it may be possible to get an error
message by calling read().  If no message is available, ENODATA will be
reported.

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

 arch/x86/entry/syscalls/syscall_32.tbl |    1 
 arch/x86/entry/syscalls/syscall_64.tbl |    1 
 fs/namespace.c                         |   99 ++++++++++++++++++++++++++++++++
 include/linux/lsm_hooks.h              |    6 ++
 include/linux/security.h               |    6 ++
 include/linux/syscalls.h               |    2 +
 kernel/sys_ni.c                        |    1 
 security/security.c                    |    5 ++
 security/selinux/hooks.c               |   13 ++++
 9 files changed, 134 insertions(+)

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 9bf8d4c62f85..abe6ea95e0e6 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -392,3 +392,4 @@
 383	i386	statx			sys_statx
 384	i386	arch_prctl		sys_arch_prctl			compat_sys_arch_prctl
 385	i386	fsopen			sys_fsopen
+386	i386	fsmount			sys_fsmount
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 9b198c5fc412..0977c5079831 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -340,6 +340,7 @@
 331	common	pkey_free		sys_pkey_free
 332	common	statx			sys_statx
 333	common	fsopen			sys_fsopen
+334	common	fsmount			sys_fsmount
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/namespace.c b/fs/namespace.c
index 91f8a07532cd..cbac401e12a1 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -3250,6 +3250,105 @@ vfs_submount_sc(const struct dentry *mountpoint, struct sb_config *sc)
 EXPORT_SYMBOL_GPL(vfs_submount_sc);
 
 /*
+ * Mount a new, prepared superblock (specified by fs_fd) on the location
+ * specified by dfd and dir_name.  dfd can be AT_FDCWD, a dir fd or a container
+ * fd.  This cannot be used for binding, moving or remounting mounts.
+ */
+SYSCALL_DEFINE5(fsmount, int, fs_fd, int, dfd, const char __user *, dir_name,
+		unsigned int, at_flags, unsigned int, flags)
+{
+	struct sb_config *sc;
+	struct inode *inode;
+	struct path mountpoint;
+	struct fd f;
+	unsigned int lookup_flags, mnt_flags = 0;
+	long ret;
+
+	if ((at_flags & ~(AT_SYMLINK_NOFOLLOW | AT_NO_AUTOMOUNT |
+			  AT_EMPTY_PATH)) != 0)
+		return -EINVAL;
+
+	if (flags & ~(MS_RDONLY | MS_NOSUID | MS_NODEV | MS_NOEXEC |
+		      MS_NOATIME | MS_NODIRATIME | MS_RELATIME | MS_STRICTATIME))
+		return -EINVAL;
+
+	if (flags & MS_RDONLY)
+		mnt_flags |= MNT_READONLY;
+	if (flags & MS_NOSUID)
+		mnt_flags |= MNT_NOSUID;
+	if (flags & MS_NODEV)
+		mnt_flags |= MNT_NODEV;
+	if (flags & MS_NOEXEC)
+		mnt_flags |= MNT_NOEXEC;
+	if (flags & MS_NODIRATIME)
+		mnt_flags |= MNT_NODIRATIME;
+
+	if (flags & MS_STRICTATIME) {
+		if (flags & MS_NOATIME)
+			return -EINVAL;
+	} else if (flags & MS_NOATIME) {
+		mnt_flags |= MNT_NOATIME;
+	} else {
+		mnt_flags |= MNT_RELATIME;
+	}
+
+	f = fdget(fs_fd);
+	if (!f.file)
+		return -EBADF;
+
+	ret = -EINVAL;
+	if (f.file->f_op != &fs_fs_fops)
+		goto err_fsfd;
+
+	sc = f.file->private_data;
+
+	ret = -EPERM;
+	if (!may_mount() ||
+	    ((sc->ms_flags & MS_MANDLOCK) && !may_mandlock()))
+		goto err_fsfd;
+
+	/* Prevent further changes. */
+	inode = file_inode(f.file);
+	ret = inode_lock_killable(inode);
+	if (ret < 0)
+		goto err_fsfd;
+	ret = -EBUSY;
+	if (!sc->mounted) {
+		sc->mounted = true;
+		ret = 0;
+	}
+	inode_unlock(inode);
+	if (ret < 0)
+		goto err_fsfd;
+
+	/* Find the mountpoint.  A container can be specified in dfd. */
+	lookup_flags = LOOKUP_FOLLOW | LOOKUP_AUTOMOUNT;
+	if (at_flags & AT_SYMLINK_NOFOLLOW)
+		lookup_flags &= ~LOOKUP_FOLLOW;
+	if (at_flags & AT_NO_AUTOMOUNT)
+		lookup_flags &= ~LOOKUP_AUTOMOUNT;
+	if (at_flags & AT_EMPTY_PATH)
+		lookup_flags |= LOOKUP_EMPTY;
+	ret = user_path_at(dfd, dir_name, lookup_flags, &mountpoint);
+	if (ret < 0) {
+		sb_cfg_error(sc, "VFS: Mountpoint lookup failed");
+		goto err_fsfd;
+	}
+
+	ret = security_sb_config_mountpoint(sc, &mountpoint);
+	if (ret < 0)
+		goto err_mp;
+
+	ret = do_new_mount_sc(sc, &mountpoint, mnt_flags);
+
+err_mp:
+	path_put(&mountpoint);
+err_fsfd:
+	fdput(f);
+	return ret;
+}
+
+/*
  * Return true if path is reachable from root
  *
  * namespace_sem or mount_lock is held
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 48bfd49666bc..9d974074940b 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -101,6 +101,10 @@
  *	Equivalent of sb_kern_mount, but with a superblock configuration context.
  *	@sc indicates the superblock configuration context.
  *	@src_sb indicates the new superblock.
+ * @sb_config_mountpoint:
+ *	Equivalent of sb_mount, but with an sb_config.
+ *	@sc indicates the superblock configuration context.
+ *	@mountpoint indicates the path on which the mount will take place.
  *
  * Security hooks for filesystem operations.
  *
@@ -1404,6 +1408,7 @@ union security_list_options {
 	void (*sb_config_free)(struct sb_config *sc);
 	int (*sb_config_parse_option)(struct sb_config *sc, char *opt);
 	int (*sb_config_kern_mount)(struct sb_config *sc, struct super_block *sb);
+	int (*sb_config_mountpoint)(struct sb_config *sc, struct path *mountpoint);
 
 	int (*sb_alloc_security)(struct super_block *sb);
 	void (*sb_free_security)(struct super_block *sb);
@@ -1721,6 +1726,7 @@ struct security_hook_heads {
 	struct list_head sb_config_free;
 	struct list_head sb_config_parse_option;
 	struct list_head sb_config_kern_mount;
+	struct list_head sb_config_mountpoint;
 	struct list_head sb_alloc_security;
 	struct list_head sb_free_security;
 	struct list_head sb_copy_data;
diff --git a/include/linux/security.h b/include/linux/security.h
index 36b3a6779986..3d9d41d91a99 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -230,6 +230,7 @@ int security_sb_config_dup(struct sb_config *sc, struct sb_config *src_sc);
 void security_sb_config_free(struct sb_config *sc);
 int security_sb_config_parse_option(struct sb_config *sc, char *opt);
 int security_sb_config_kern_mount(struct sb_config *sc, struct super_block *sb);
+int security_sb_config_mountpoint(struct sb_config *sc, struct path *mountpoint);
 int security_sb_alloc(struct super_block *sb);
 void security_sb_free(struct super_block *sb);
 int security_sb_copy_data(char *orig, char *copy);
@@ -548,6 +549,11 @@ static inline int security_sb_config_kern_mount(struct sb_config *sc,
 {
 	return 0;
 }
+static inline int security_sb_config_mountpoint(struct sb_config *sc,
+						struct path *mountpoint)
+{
+	return 0;
+}
 
 static inline int security_sb_alloc(struct super_block *sb)
 {
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 91ec8802ad5d..07e4f775f24d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -906,5 +906,7 @@ asmlinkage long sys_pkey_free(int pkey);
 asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
 			  unsigned mask, struct statx __user *buffer);
 asmlinkage long sys_fsopen(const char *fs_name, int containerfd, unsigned int flags);
+asmlinkage long sys_fsmount(int fsfd, int dfd, const char *path, unsigned int at_flags,
+			    unsigned int flags);
 
 #endif
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index de1dc63e7e47..a0fe764bd5dd 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -261,3 +261,4 @@ cond_syscall(sys_pkey_free);
 
 /* fd-based mount */
 cond_syscall(sys_fsopen);
+cond_syscall(sys_fsmount);
diff --git a/security/security.c b/security/security.c
index 3735fad91543..197ff60b57a7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -341,6 +341,11 @@ int security_sb_config_kern_mount(struct sb_config *sc, struct super_block *sb)
 	return call_int_hook(sb_config_kern_mount, 0, sc, sb);
 }
 
+int security_sb_config_mountpoint(struct sb_config *sc, struct path *mountpoint)
+{
+	return call_int_hook(sb_config_mountpoint, 0, sc, mountpoint);
+}
+
 int security_sb_alloc(struct super_block *sb)
 {
 	return call_int_hook(sb_alloc_security, 0, sb);
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 286207bced52..f073b94c0035 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2990,6 +2990,18 @@ static int selinux_sb_config_kern_mount(struct sb_config *sc,
 	return rc;
 }
 
+static int selinux_sb_config_mountpoint(struct sb_config *sc,
+					struct path *mountpoint)
+{
+	const struct cred *cred = current_cred();
+	int ret;
+
+	ret = path_has_perm(cred, mountpoint, FILE__MOUNTON);
+	if (ret < 0)
+		sb_cfg_error(sc, "SELinux: Mount on mountpoint not permitted");
+	return ret;
+}
+
 /* inode security operations */
 
 static int selinux_inode_alloc_security(struct inode *inode)
@@ -6323,6 +6335,7 @@ static struct security_hook_list selinux_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_config_free, selinux_sb_config_free),
 	LSM_HOOK_INIT(sb_config_parse_option, selinux_sb_config_parse_option),
 	LSM_HOOK_INIT(sb_config_kern_mount, selinux_sb_config_kern_mount),
+	LSM_HOOK_INIT(sb_config_mountpoint, selinux_sb_config_mountpoint),
 
 	LSM_HOOK_INIT(sb_alloc_security, selinux_sb_alloc_security),
 	LSM_HOOK_INIT(sb_free_security, selinux_sb_free_security),

[toc] | [prev] | [next] | [standalone]


#1641802 — [PATCH 02/21] Clean up whitespace in fs/namespace.c [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:20 +0200
Subject[PATCH 02/21] Clean up whitespace in fs/namespace.c [ver #3]
Message-ID<tHpWW-1pB-27@gated-at.bofh.it>
In reply to#1641798
Clean up line terminal whitespace in fs/namespace.c.

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

 fs/namespace.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 8bd3e4d448b9..c076787871e7 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -1656,7 +1656,7 @@ void __detach_mounts(struct dentry *dentry)
 	namespace_unlock();
 }
 
-/* 
+/*
  * Is the caller allowed to modify his namespace?
  */
 static inline bool may_mount(void)
@@ -2210,7 +2210,7 @@ static int do_loopback(struct path *path, const char *old_name,
 
 	err = -EINVAL;
 	if (mnt_ns_loop(old_path.dentry))
-		goto out; 
+		goto out;
 
 	mp = lock_mount(path);
 	err = PTR_ERR(mp);

[toc] | [prev] | [next] | [standalone]


#1641804 — [PATCH 01/21] Provide a function to create a NUL-terminated string from unterminated data [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:20 +0200
Subject[PATCH 01/21] Provide a function to create a NUL-terminated string from unterminated data [ver #3]
Message-ID<tHpWW-1pB-31@gated-at.bofh.it>
In reply to#1641798
Provide a function, kmemdup_nul(), that will create a NUL-terminated string
from an unterminated character array where the length is known in advance.

This is better than kstrndup() in situations where we already know the
string length as the strnlen() in kstrndup() is superfluous.

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

 include/linux/string.h |    1 +
 mm/util.c              |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/linux/string.h b/include/linux/string.h
index 537918f8a98e..3dd944cfe171 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -131,6 +131,7 @@ extern char *kstrdup(const char *s, gfp_t gfp) __malloc;
 extern const char *kstrdup_const(const char *s, gfp_t gfp);
 extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
+extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp);
 
 extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
 extern void argv_free(char **argv);
diff --git a/mm/util.c b/mm/util.c
index 464df3489903..cc9f0a2810bd 100644
--- a/mm/util.c
+++ b/mm/util.c
@@ -83,6 +83,8 @@ EXPORT_SYMBOL(kstrdup_const);
  * @s: the string to duplicate
  * @max: read at most @max chars from @s
  * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ *
+ * Note: Use kmemdup_nul() instead if the size is known exactly.
  */
 char *kstrndup(const char *s, size_t max, gfp_t gfp)
 {
@@ -121,6 +123,28 @@ void *kmemdup(const void *src, size_t len, gfp_t gfp)
 EXPORT_SYMBOL(kmemdup);
 
 /**
+ * kmemdup_nul - Create a NUL-terminated string from unterminated data
+ * @s: The data to stringify
+ * @len: The size of the data
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ */
+char *kmemdup_nul(const char *s, size_t len, gfp_t gfp)
+{
+	char *buf;
+
+	if (!s)
+		return NULL;
+
+	buf = kmalloc_track_caller(len + 1, gfp);
+	if (buf) {
+		memcpy(buf, s, len);
+		buf[len] = '\0';
+	}
+	return buf;
+}
+EXPORT_SYMBOL(kmemdup_nul);
+
+/**
  * memdup_user - duplicate memory region from user space
  *
  * @src: source address in user space

[toc] | [prev] | [next] | [standalone]


#1641805 — [PATCH 05/21] VFS: Provide empty name qstr [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:20 +0200
Subject[PATCH 05/21] VFS: Provide empty name qstr [ver #3]
Message-ID<tHpWW-1pB-35@gated-at.bofh.it>
In reply to#1641798
Provide an empty name (ie. "") qstr for general use.

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

 fs/dcache.c            |    8 ++++++--
 fs/gfs2/dir.c          |    3 +--
 fs/namei.c             |    3 +--
 fs/nsfs.c              |    3 +--
 fs/pipe.c              |    3 +--
 include/linux/dcache.h |    5 +++++
 6 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/fs/dcache.c b/fs/dcache.c
index cddf39777835..d3da2a3ffd73 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -90,6 +90,11 @@ EXPORT_SYMBOL(rename_lock);
 
 static struct kmem_cache *dentry_cache __read_mostly;
 
+const struct qstr empty_name = QSTR_INIT("", 0);
+EXPORT_SYMBOL(empty_name);
+const struct qstr slash_name = QSTR_INIT("/", 1);
+EXPORT_SYMBOL(slash_name);
+
 /*
  * This is the single most critical data structure when it comes
  * to the dcache: the hashtable for lookups. Somebody should try
@@ -1580,8 +1585,7 @@ struct dentry *__d_alloc(struct super_block *sb, const struct qstr *name)
 	 */
 	dentry->d_iname[DNAME_INLINE_LEN-1] = 0;
 	if (unlikely(!name)) {
-		static const struct qstr anon = QSTR_INIT("/", 1);
-		name = &anon;
+		name = &slash_name;
 		dname = dentry->d_iname;
 	} else if (name->len > DNAME_INLINE_LEN-1) {
 		size_t size = offsetof(struct external_name, name[1]);
diff --git a/fs/gfs2/dir.c b/fs/gfs2/dir.c
index 79113219be5f..a5dfff6a033e 100644
--- a/fs/gfs2/dir.c
+++ b/fs/gfs2/dir.c
@@ -872,7 +872,6 @@ static struct gfs2_leaf *new_leaf(struct inode *inode, struct buffer_head **pbh,
 	struct buffer_head *bh;
 	struct gfs2_leaf *leaf;
 	struct gfs2_dirent *dent;
-	struct qstr name = { .name = "" };
 	struct timespec tv = current_time(inode);
 
 	error = gfs2_alloc_blocks(ip, &bn, &n, 0, NULL);
@@ -896,7 +895,7 @@ static struct gfs2_leaf *new_leaf(struct inode *inode, struct buffer_head **pbh,
 	leaf->lf_sec = cpu_to_be64(tv.tv_sec);
 	memset(leaf->lf_reserved2, 0, sizeof(leaf->lf_reserved2));
 	dent = (struct gfs2_dirent *)(leaf+1);
-	gfs2_qstr2dirent(&name, bh->b_size - sizeof(struct gfs2_leaf), dent);
+	gfs2_qstr2dirent(&empty_name, bh->b_size - sizeof(struct gfs2_leaf), dent);
 	*pbh = bh;
 	return leaf;
 }
diff --git a/fs/namei.c b/fs/namei.c
index 6571a5f5112e..0d35760fee00 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3400,7 +3400,6 @@ static int do_last(struct nameidata *nd,
 
 struct dentry *vfs_tmpfile(struct dentry *dentry, umode_t mode, int open_flag)
 {
-	static const struct qstr name = QSTR_INIT("/", 1);
 	struct dentry *child = NULL;
 	struct inode *dir = dentry->d_inode;
 	struct inode *inode;
@@ -3414,7 +3413,7 @@ struct dentry *vfs_tmpfile(struct dentry *dentry, umode_t mode, int open_flag)
 	if (!dir->i_op->tmpfile)
 		goto out_err;
 	error = -ENOMEM;
-	child = d_alloc(dentry, &name);
+	child = d_alloc(dentry, &slash_name);
 	if (unlikely(!child))
 		goto out_err;
 	error = dir->i_op->tmpfile(dir, child, mode);
diff --git a/fs/nsfs.c b/fs/nsfs.c
index f3db56e83dd2..08127a2b8559 100644
--- a/fs/nsfs.c
+++ b/fs/nsfs.c
@@ -53,7 +53,6 @@ static void nsfs_evict(struct inode *inode)
 static void *__ns_get_path(struct path *path, struct ns_common *ns)
 {
 	struct vfsmount *mnt = nsfs_mnt;
-	struct qstr qname = { .name = "", };
 	struct dentry *dentry;
 	struct inode *inode;
 	unsigned long d;
@@ -85,7 +84,7 @@ static void *__ns_get_path(struct path *path, struct ns_common *ns)
 	inode->i_fop = &ns_file_operations;
 	inode->i_private = ns;
 
-	dentry = d_alloc_pseudo(mnt->mnt_sb, &qname);
+	dentry = d_alloc_pseudo(mnt->mnt_sb, &empty_name);
 	if (!dentry) {
 		iput(inode);
 		return ERR_PTR(-ENOMEM);
diff --git a/fs/pipe.c b/fs/pipe.c
index 73b84baf58f8..97e5be897753 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -739,13 +739,12 @@ int create_pipe_files(struct file **res, int flags)
 	struct inode *inode = get_pipe_inode();
 	struct file *f;
 	struct path path;
-	static struct qstr name = { .name = "" };
 
 	if (!inode)
 		return -ENFILE;
 
 	err = -ENOMEM;
-	path.dentry = d_alloc_pseudo(pipe_mnt->mnt_sb, &name);
+	path.dentry = d_alloc_pseudo(pipe_mnt->mnt_sb, &empty_name);
 	if (!path.dentry)
 		goto err_inode;
 	path.mnt = mntget(pipe_mnt);
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index d2e38dc6172c..3f65a4fa72ed 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -55,6 +55,11 @@ struct qstr {
 
 #define QSTR_INIT(n,l) { { { .len = l } }, .name = n }
 
+extern const char empty_string[];
+extern const struct qstr empty_name;
+extern const char slash_string[];
+extern const struct qstr slash_name;
+
 struct dentry_stat_t {
 	long nr_dentry;
 	long nr_unused;

[toc] | [prev] | [next] | [standalone]


#1641806 — [PATCH 07/21] Implement fsopen() to prepare for a mount [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 07/21] Implement fsopen() to prepare for a mount [ver #3]
Message-ID<tHq6B-1u3-1@gated-at.bofh.it>
In reply to#1641798
Provide an fsopen() system call that starts the process of preparing to
mount, using an fd as a context handle.  fsopen() is given the name of the
filesystem that will be used:

	int mfd = fsopen(const char *fsname, int reserved,
			 int open_flags);

where reserved should be -1 for the moment (it will be used to pass the
namespace information in future) and open_flags can be 0 or O_CLOEXEC.

For example:

	mfd = fsopen("ext4", -1, O_CLOEXEC);
	write(mfd, "s /dev/sdb1"); // note I'm ignoring write's length arg
	write(mfd, "o noatime");
	write(mfd, "o acl");
	write(mfd, "o user_attr");
	write(mfd, "o iversion");
	write(mfd, "o ");
	write(mfd, "r /my/container"); // root inside the fs
	fsmount(mfd, container_fd, "/mnt", AT_NO_FOLLOW);

	mfd = fsopen("afs", -1);
	write(mfd, "s %grand.central.org:root.cell");
	write(mfd, "o cell=grand.central.org");
	write(mfd, "r /");
	fsmount(mfd, AT_FDCWD, "/mnt", 0);

If an error is reported at any step, an error message may be available to be
read() back (ENODATA will be reported if there isn't an error available) in
the form:

	"e <subsys>:<problem>"
	"e SELinux:Mount on mountpoint not permitted"

Once fsmount() has been called, further write() calls will incur EBUSY,
even if the fsmount() fails.  read() is still possible to retrieve error
information.

The fsopen() syscall creates a mount context and hangs it of the fd that it
returns.

Netlink is not used because it is optional.

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

 arch/x86/entry/syscalls/syscall_32.tbl |    1 
 arch/x86/entry/syscalls/syscall_64.tbl |    1 
 fs/Makefile                            |    2 
 fs/fsopen.c                            |  279 ++++++++++++++++++++++++++++++++
 include/linux/syscalls.h               |    1 
 include/uapi/linux/magic.h             |    1 
 kernel/sys_ni.c                        |    3 
 7 files changed, 287 insertions(+), 1 deletion(-)
 create mode 100644 fs/fsopen.c

diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 448ac2161112..9bf8d4c62f85 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -391,3 +391,4 @@
 382	i386	pkey_free		sys_pkey_free
 383	i386	statx			sys_statx
 384	i386	arch_prctl		sys_arch_prctl			compat_sys_arch_prctl
+385	i386	fsopen			sys_fsopen
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index 5aef183e2f85..9b198c5fc412 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -339,6 +339,7 @@
 330	common	pkey_alloc		sys_pkey_alloc
 331	common	pkey_free		sys_pkey_free
 332	common	statx			sys_statx
+333	common	fsopen			sys_fsopen
 
 #
 # x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/Makefile b/fs/Makefile
index 8f5142525866..b8fcf48b0400 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -12,7 +12,7 @@ obj-y :=	open.o read_write.o file_table.o super.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o splice.o sync.o utimes.o \
 		stack.o fs_struct.o statfs.o fs_pin.o nsfs.o \
-		sb_config.o
+		sb_config.o fsopen.o
 
 ifeq ($(CONFIG_BLOCK),y)
 obj-y +=	buffer.o block_dev.o direct-io.o mpage.o
diff --git a/fs/fsopen.c b/fs/fsopen.c
new file mode 100644
index 000000000000..a4e9d5a7ce2b
--- /dev/null
+++ b/fs/fsopen.c
@@ -0,0 +1,279 @@
+/* Filesystem access-by-fd.
+ *
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/sb_config.h>
+#include <linux/mount.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/file.h>
+#include <linux/magic.h>
+#include <linux/syscalls.h>
+
+static struct vfsmount *fs_fs_mnt __read_mostly;
+
+static int fs_fs_release(struct inode *inode, struct file *file)
+{
+	struct sb_config *sc = file->private_data;
+
+	file->private_data = NULL;
+
+	put_sb_config(sc);
+	return 0;
+}
+
+/*
+ * Read any error message back from the fd.  Will be prefixed by "e ".
+ */
+static ssize_t fs_fs_read(struct file *file, char __user *_buf, size_t len, loff_t *pos)
+{
+	struct sb_config *sc = file->private_data;
+	const char *msg;
+	size_t mlen;
+
+	msg = READ_ONCE(sc->error_msg);
+	if (!msg)
+		return -ENODATA;
+
+	mlen = strlen(msg);
+	if (mlen + 2 > len)
+		return -ETOOSMALL;
+	if (copy_to_user(_buf, "e ", 2) != 0 ||
+	    copy_to_user(_buf + 2, msg, mlen) != 0)
+		return -EFAULT;
+	return mlen + 2;
+}
+
+/*
+ * Userspace writes configuration data to the fd and we parse it here.  For the
+ * moment, we assume a single option per write.  Each line written is of the form
+ *
+ *	<option_type><space><stuff...>
+ *
+ *	d /dev/sda1				-- Device name
+ *	o noatime				-- Option without value
+ *	o cell=grand.central.org		-- Option with value
+ *	r /					-- Dir within device to mount
+ */
+static ssize_t fs_fs_write(struct file *file,
+			   const char __user *_buf, size_t len, loff_t *pos)
+{
+	struct sb_config *sc = file->private_data;
+	struct inode *inode = file_inode(file);
+	char opt[2], *data;
+	ssize_t ret;
+
+	if (len < 3 || len > 4095)
+		return -EINVAL;
+
+	if (copy_from_user(opt, _buf, 2) != 0)
+		return -EFAULT;
+	switch (opt[0]) {
+	case 's':
+	case 'o':
+		break;
+	default:
+		return sb_cfg_inval(sc, "VFS: Unsupported write spec");
+	}
+	if (opt[1] != ' ')
+		return sb_cfg_inval(sc, "VFS: Unsupported write spec");
+
+	data = memdup_user_nul(_buf + 2, len - 2);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	/* From this point onwards we need to lock the fd against someone
+	 * trying to mount it.
+	 */
+	ret = inode_lock_killable(inode);
+	if (ret < 0)
+		goto err_free;
+
+	ret = -EBUSY;
+	if (sc->mounted)
+		goto err_unlock;
+
+	ret = -EINVAL;
+	switch (opt[0]) {
+	case 's':
+		if (sc->device)
+			goto err_unlock;
+		sc->device = data;
+		data = NULL;
+		break;
+
+	case 'o':
+		ret = vfs_parse_mount_option(sc, data);
+		if (ret < 0)
+			goto err_unlock;
+		break;
+
+	default:
+		goto err_unlock;
+	}
+
+	ret = len;
+err_unlock:
+	inode_unlock(inode);
+err_free:
+	kfree(data);
+	return ret;
+}
+
+const struct file_operations fs_fs_fops = {
+	.read		= fs_fs_read,
+	.write		= fs_fs_write,
+	.release	= fs_fs_release,
+	.llseek		= no_llseek,
+};
+
+/*
+ * Indicate the name we want to display the filesystem file as.
+ */
+static char *fs_fs_dname(struct dentry *dentry, char *buffer, int buflen)
+{
+	return dynamic_dname(dentry, buffer, buflen, "fs:[%lu]",
+			     d_inode(dentry)->i_ino);
+}
+
+static const struct dentry_operations fs_fs_dentry_operations = {
+	.d_dname	= fs_fs_dname,
+};
+
+/*
+ * Create a file that can be used to configure a new mount.
+ */
+static struct file *create_fs_file(struct sb_config *sc)
+{
+	struct inode *inode;
+	struct file *f;
+	struct path path;
+	int ret;
+
+	inode = alloc_anon_inode(fs_fs_mnt->mnt_sb);
+	if (!inode)
+		return ERR_PTR(-ENFILE);
+	inode->i_fop = &fs_fs_fops;
+
+	ret = -ENOMEM;
+	path.dentry = d_alloc_pseudo(fs_fs_mnt->mnt_sb, &empty_name);
+	if (!path.dentry)
+		goto err_inode;
+	path.mnt = mntget(fs_fs_mnt);
+
+	d_instantiate(path.dentry, inode);
+
+	f = alloc_file(&path, FMODE_READ | FMODE_WRITE, &fs_fs_fops);
+	if (IS_ERR(f)) {
+		ret = PTR_ERR(f);
+		goto err_file;
+	}
+
+	f->private_data = sc;
+	return f;
+
+err_file:
+	path_put(&path);
+	return ERR_PTR(ret);
+
+err_inode:
+	iput(inode);
+	return ERR_PTR(ret);
+}
+
+ const struct super_operations fs_fs_ops = {
+	.drop_inode	= generic_delete_inode,
+	.destroy_inode	= free_inode_nonrcu,
+	.statfs		= simple_statfs,
+};
+
+static struct dentry *fs_fs_mount(struct file_system_type *fs_type,
+				  int flags, const char *dev_name,
+				  void *data)
+{
+	return mount_pseudo(fs_type, "fs_fs:", &fs_fs_ops,
+			    &fs_fs_dentry_operations, FS_FS_MAGIC);
+}
+
+static struct file_system_type fs_fs_type = {
+	.name		= "fs_fs",
+	.mount		= fs_fs_mount,
+	.kill_sb	= kill_anon_super,
+};
+
+static int __init init_fs_fs(void)
+{
+	int ret;
+
+	ret = register_filesystem(&fs_fs_type);
+	if (ret < 0)
+		panic("Cannot register fs_fs\n");
+
+	fs_fs_mnt = kern_mount(&fs_fs_type);
+	if (IS_ERR(fs_fs_mnt))
+		panic("Cannot mount fs_fs: %ld\n", PTR_ERR(fs_fs_mnt));
+	return 0;
+}
+
+fs_initcall(init_fs_fs);
+
+/*
+ * Open a filesystem by name so that it can be configured for mounting.
+ *
+ * We are allowed to specify a container in which the filesystem will be
+ * opened, thereby indicating which namespaces will be used (notably, which
+ * network namespace will be used for network filesystems).
+ */
+SYSCALL_DEFINE3(fsopen, const char __user *, _fs_name, int, reserved,
+		unsigned int, flags)
+{
+	struct sb_config *sc;
+	struct file *file;
+	const char *fs_name;
+	int fd, ret;
+
+	if (flags & ~O_CLOEXEC || reserved != -1)
+		return -EINVAL;
+
+	fs_name = strndup_user(_fs_name, PAGE_SIZE);
+	if (IS_ERR(fs_name))
+		return PTR_ERR(fs_name);
+
+	sc = vfs_new_sb_config(fs_name);
+	kfree(fs_name);
+	if (IS_ERR(sc))
+		return PTR_ERR(sc);
+
+	ret = -ENOTSUPP;
+	if (!sc->ops)
+		goto err_sc;
+
+	file = create_fs_file(sc);
+	if (IS_ERR(file)) {
+		ret = PTR_ERR(file);
+		goto err_sc;
+	}
+
+	ret = get_unused_fd_flags(flags & O_CLOEXEC);
+	if (ret < 0)
+		goto err_file;
+
+	fd = ret;
+	fd_install(fd, file);
+	return fd;
+
+err_file:
+	fput(file);
+	return ret;
+
+err_sc:
+	put_sb_config(sc);
+	return ret;
+}
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 980c3c9b06f8..91ec8802ad5d 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -905,5 +905,6 @@ asmlinkage long sys_pkey_alloc(unsigned long flags, unsigned long init_val);
 asmlinkage long sys_pkey_free(int pkey);
 asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
 			  unsigned mask, struct statx __user *buffer);
+asmlinkage long sys_fsopen(const char *fs_name, int containerfd, unsigned int flags);
 
 #endif
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index e230af2e6855..88ae83492f7c 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -84,5 +84,6 @@
 #define UDF_SUPER_MAGIC		0x15013346
 #define BALLOON_KVM_MAGIC	0x13661366
 #define ZSMALLOC_MAGIC		0x58295829
+#define FS_FS_MAGIC		0x66736673
 
 #endif /* __LINUX_MAGIC_H__ */
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index 8acef8576ce9..de1dc63e7e47 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -258,3 +258,6 @@ cond_syscall(sys_membarrier);
 cond_syscall(sys_pkey_mprotect);
 cond_syscall(sys_pkey_alloc);
 cond_syscall(sys_pkey_free);
+
+/* fd-based mount */
+cond_syscall(sys_fsopen);

[toc] | [prev] | [next] | [standalone]


#1641807 — [PATCH 18/21] NFS: Do some tidying of the parsing code [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 18/21] NFS: Do some tidying of the parsing code [ver #3]
Message-ID<tHq6C-1u3-7@gated-at.bofh.it>
In reply to#1641798
Do some tidying of the parsing code, including:

 (*) Returning 0/error rather than true/false.

 (*) Putting the nfs_sb_config pointer first in some arg lists.

 (*) Unwrap some lines that will now fit on one line.

 (*) Provide unioned sockaddr/sockaddr_storage fields to avoid casts.

 (*) nfs_parse_devname() can paste its return values directly into the
     nfs_sb_config struct as that's where the caller puts them.
---

 fs/nfs/internal.h |   16 +++++--
 fs/nfs/mount.c    |  128 +++++++++++++++++++++++------------------------------
 fs/nfs/super.c    |    2 -
 3 files changed, 67 insertions(+), 79 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index 52242240933f..79e77ff2061c 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -86,11 +86,11 @@ struct nfs_client_initdata {
  * In-kernel mount arguments
  */
 struct nfs_sb_config {
-	int			flags;
+	unsigned int		flags;		/* NFS{,4}_MOUNT_* flags */
 	unsigned int		rsize, wsize;
 	unsigned int		timeo, retrans;
-	unsigned int		acregmin, acregmax,
-				acdirmin, acdirmax;
+	unsigned int		acregmin, acregmax;
+	unsigned int		acdirmin, acdirmax;
 	unsigned int		namlen;
 	unsigned int		options;
 	unsigned int		bsize;
@@ -106,7 +106,10 @@ struct nfs_sb_config {
 	bool			sloppy;
 
 	struct {
-		struct sockaddr_storage	address;
+		union {
+			struct sockaddr	address;
+			struct sockaddr_storage	_address;
+		};
 		size_t			addrlen;
 		char			*hostname;
 		u32			version;
@@ -115,7 +118,10 @@ struct nfs_sb_config {
 	} mount_server;
 
 	struct {
-		struct sockaddr_storage	address;
+		union {
+			struct sockaddr	address;
+			struct sockaddr_storage	_address;
+		};
 		size_t			addrlen;
 		char			*hostname;
 		char			*export_path;
diff --git a/fs/nfs/mount.c b/fs/nfs/mount.c
index bebc589badbb..e0c3381ad5c5 100644
--- a/fs/nfs/mount.c
+++ b/fs/nfs/mount.c
@@ -341,8 +341,9 @@ static void nfs_set_mount_transport_protocol(struct nfs_sb_config *cfg)
  * Add 'flavor' to 'auth_info' if not already present.
  * Returns true if 'flavor' ends up in the list, false otherwise
  */
-static bool nfs_auth_info_add(struct nfs_auth_info *auth_info,
-			      rpc_authflavor_t flavor)
+static int nfs_auth_info_add(struct nfs_sb_config *cfg,
+			     struct nfs_auth_info *auth_info,
+			     rpc_authflavor_t flavor)
 {
 	unsigned int i;
 	unsigned int max_flavor_len = ARRAY_SIZE(auth_info->flavors);
@@ -350,26 +351,27 @@ static bool nfs_auth_info_add(struct nfs_auth_info *auth_info,
 	/* make sure this flavor isn't already in the list */
 	for (i = 0; i < auth_info->flavor_len; i++) {
 		if (flavor == auth_info->flavors[i])
-			return true;
+			return 0;
 	}
 
 	if (auth_info->flavor_len + 1 >= max_flavor_len) {
 		dfprintk(MOUNT, "NFS: too many sec= flavors\n");
-		return false;
+		return -EINVAL;
 	}
 
 	auth_info->flavors[auth_info->flavor_len++] = flavor;
-	return true;
+	return 0;
 }
 
 /*
  * Parse the value of the 'sec=' option.
  */
-static int nfs_parse_security_flavors(char *value, struct nfs_sb_config *cfg)
+static int nfs_parse_security_flavors(struct nfs_sb_config *cfg, char *value)
 {
 	substring_t args[MAX_OPT_ARGS];
 	rpc_authflavor_t pseudoflavor;
 	char *p;
+	int ret;
 
 	dfprintk(MOUNT, "NFS: parsing sec=%s option\n", value);
 
@@ -411,19 +413,20 @@ static int nfs_parse_security_flavors(char *value, struct nfs_sb_config *cfg)
 		default:
 			dfprintk(MOUNT,
 				 "NFS: sec= option '%s' not recognized\n", p);
-			return 0;
+			return -EINVAL;
 		}
 
-		if (!nfs_auth_info_add(&cfg->auth_info, pseudoflavor))
-			return 0;
+		ret = nfs_auth_info_add(cfg, &cfg->auth_info, pseudoflavor);
+		if (ret < 0)
+			return ret;
 	}
 
-	return 1;
+	return 0;
 }
 
-static int nfs_parse_version_string(char *string,
-		struct nfs_sb_config *cfg,
-		substring_t *args)
+static int nfs_parse_version_string(struct nfs_sb_config *cfg,
+				    char *string,
+				    substring_t *args)
 {
 	cfg->flags &= ~NFS_MOUNT_VER3;
 	switch (match_token(string, nfs_vers_tokens, args)) {
@@ -454,9 +457,10 @@ static int nfs_parse_version_string(char *string,
 		cfg->minorversion = 2;
 		break;
 	default:
-		return 0;
+		dfprintk(MOUNT, "NFS:   Unsupported NFS version\n");
+		return -EINVAL;
 	}
-	return 1;
+	return 0;
 }
 
 static int nfs_get_option_str(substring_t args[], char **option)
@@ -495,7 +499,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 {
 	substring_t args[MAX_OPT_ARGS];
 	char *string;
-	int rc, token;
+	int ret, token;
 
 	dfprintk(MOUNT, "NFS:   parsing nfs mount option '%s'\n", p);
 
@@ -530,13 +534,11 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		break;
 	case Opt_lock:
 		cfg->flags &= ~NFS_MOUNT_NONLM;
-		cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
-				NFS_MOUNT_LOCAL_FCNTL);
+		cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL);
 		break;
 	case Opt_nolock:
 		cfg->flags |= NFS_MOUNT_NONLM;
-		cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK |
-			       NFS_MOUNT_LOCAL_FCNTL);
+		cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK | NFS_MOUNT_LOCAL_FCNTL);
 		break;
 	case Opt_udp:
 		cfg->flags &= ~NFS_MOUNT_TCP;
@@ -669,29 +671,25 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		rc = nfs_parse_version_string(string, cfg, args);
+		ret = nfs_parse_version_string(cfg, string, args);
 		kfree(string);
-		if (!rc)
-			goto out_invalid_value;
+		if (ret < 0)
+			return ret;
 		break;
 	case Opt_sec:
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		rc = nfs_parse_security_flavors(string, cfg);
+		ret = nfs_parse_security_flavors(cfg, string);
 		kfree(string);
-		if (!rc) {
-			dfprintk(MOUNT, "NFS:   unrecognized "
-				 "security flavor\n");
-			return -EINVAL;
-		}
+		if (ret < 0)
+			return ret;
 		break;
 	case Opt_proto:
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		token = match_token(string,
-				    nfs_xprt_protocol_tokens, args);
+		token = match_token(string, nfs_xprt_protocol_tokens, args);
 
 		cfg->protofamily = AF_INET;
 		switch (token) {
@@ -716,9 +714,8 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			xprt_load_transport(string);
 			break;
 		default:
-			dfprintk(MOUNT, "NFS:   unrecognized "
-				 "transport protocol\n");
 			kfree(string);
+			dfprintk(MOUNT, "NFS:   unrecognized transport protocol\n");
 			return -EINVAL;
 		}
 		kfree(string);
@@ -727,8 +724,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		token = match_token(string,
-				    nfs_xprt_protocol_tokens, args);
+		token = match_token(string, nfs_xprt_protocol_tokens, args);
 		kfree(string);
 
 		cfg->mountfamily = AF_INET;
@@ -745,8 +741,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			break;
 		case Opt_xprt_rdma: /* not used for side protocols */
 		default:
-			dfprintk(MOUNT, "NFS:   unrecognized "
-				 "transport protocol\n");
+			dfprintk(MOUNT, "NFS:   unrecognized transport protocol\n");
 			return -EINVAL;
 		}
 		break;
@@ -756,9 +751,8 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			goto out_nomem;
 		cfg->nfs_server.addrlen =
 			rpc_pton(cfg->net, string, strlen(string),
-				 (struct sockaddr *)
 				 &cfg->nfs_server.address,
-				 sizeof(cfg->nfs_server.address));
+				 sizeof(cfg->nfs_server._address));
 		kfree(string);
 		if (cfg->nfs_server.addrlen == 0)
 			goto out_invalid_address;
@@ -768,8 +762,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			goto out_nomem;
 		break;
 	case Opt_mounthost:
-		if (nfs_get_option_str(args,
-				       &cfg->mount_server.hostname))
+		if (nfs_get_option_str(args, &cfg->mount_server.hostname))
 			goto out_nomem;
 		break;
 	case Opt_mountaddr:
@@ -778,9 +771,8 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			goto out_nomem;
 		cfg->mount_server.addrlen =
 			rpc_pton(cfg->net, string, strlen(string),
-				 (struct sockaddr *)
 				 &cfg->mount_server.address,
-				 sizeof(cfg->mount_server.address));
+				 sizeof(cfg->mount_server._address));
 		kfree(string);
 		if (cfg->mount_server.addrlen == 0)
 			goto out_invalid_address;
@@ -789,8 +781,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		token = match_token(string,
-				    nfs_lookupcache_tokens, args);
+		token = match_token(string, nfs_lookupcache_tokens, args);
 		kfree(string);
 		switch (token) {
 		case Opt_lookupcache_all:
@@ -804,10 +795,9 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 			cfg->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE;
 			break;
 		default:
-			dfprintk(MOUNT, "NFS:   invalid "
-				 "lookupcache argument\n");
+			dfprintk(MOUNT, "NFS:   invalid lookupcache argument\n");
 			return -EINVAL;
-		};
+		}
 		break;
 	case Opt_fscache_uniq:
 		if (nfs_get_option_str(args, &cfg->fscache_uniq))
@@ -818,8 +808,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		string = match_strdup(args);
 		if (string == NULL)
 			goto out_nomem;
-		token = match_token(string, nfs_local_lock_tokens,
-				    args);
+		token = match_token(string, nfs_local_lock_tokens, args);
 		kfree(string);
 		switch (token) {
 		case Opt_local_lock_all:
@@ -837,8 +826,7 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 					NFS_MOUNT_LOCAL_FCNTL);
 			break;
 		default:
-			dfprintk(MOUNT, "NFS:	invalid	"
-				 "local_lock argument\n");
+			dfprintk(MOUNT, "NFS:	invalid	local_lock argument\n");
 			return -EINVAL;
 		};
 		break;
@@ -852,13 +840,11 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		break;
 	case Opt_userspace:
 	case Opt_deprecated:
-		dfprintk(MOUNT, "NFS:   ignoring mount option "
-			 "'%s'\n", p);
+		dfprintk(MOUNT, "NFS:   ignoring mount option '%s'\n", p);
 		break;
 
 	default:
-		dfprintk(MOUNT, "NFS:   unrecognized mount option "
-			 "'%s'\n", p);
+		dfprintk(MOUNT, "NFS:   unrecognized mount option '%s'\n", p);
 		return -EINVAL;
 	}
 
@@ -928,15 +914,15 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 	 * families in the addr=/mountaddr= options.
 	 */
 	if (cfg->protofamily != AF_UNSPEC &&
-	    cfg->protofamily != cfg->nfs_server.address.ss_family)
+	    cfg->protofamily != cfg->nfs_server.address.sa_family)
 		goto out_proto_mismatch;
 
 	if (cfg->mountfamily != AF_UNSPEC) {
 		if (cfg->mount_server.addrlen) {
-			if (cfg->mountfamily != cfg->mount_server.address.ss_family)
+			if (cfg->mountfamily != cfg->mount_server.address.sa_family)
 				goto out_mountproto_mismatch;
 		} else {
-			if (cfg->mountfamily != cfg->nfs_server.address.ss_family)
+			if (cfg->mountfamily != cfg->nfs_server.address.sa_family)
 				goto out_mountproto_mismatch;
 		}
 	}
@@ -976,9 +962,9 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
  *
  * Note: caller frees hostname and export path, even on error.
  */
-static int nfs_parse_devname(const char *dev_name,
-			     char **hostname, size_t maxnamlen,
-			     char **export_path, size_t maxpathlen)
+static int nfs_parse_devname(struct nfs_sb_config *cfg,
+			     const char *dev_name,
+			     size_t maxnamlen, size_t maxpathlen)
 {
 	size_t len;
 	char *end;
@@ -1009,17 +995,17 @@ static int nfs_parse_devname(const char *dev_name,
 		goto out_hostname;
 
 	/* N.B. caller will free nfs_server.hostname in all cases */
-	*hostname = kstrndup(dev_name, len, GFP_KERNEL);
-	if (*hostname == NULL)
+	cfg->nfs_server.hostname = kmemdup_nul(dev_name, len, GFP_KERNEL);
+	if (!cfg->nfs_server.hostname)
 		goto out_nomem;
 	len = strlen(++end);
 	if (len > maxpathlen)
 		goto out_path;
-	*export_path = kstrndup(end, len, GFP_KERNEL);
-	if (!*export_path)
+	cfg->nfs_server.export_path = kmemdup_nul(end, len, GFP_KERNEL);
+	if (!cfg->nfs_server.export_path)
 		goto out_nomem;
 
-	dfprintk(MOUNT, "NFS: MNTPATH: '%s'\n", *export_path);
+	dfprintk(MOUNT, "NFS: MNTPATH: '%s'\n", cfg->nfs_server.export_path);
 	return 0;
 
 out_bad_devname:
@@ -1040,7 +1026,7 @@ static int nfs_parse_devname(const char *dev_name,
 }
 
 /*
- * Validate the NFS2/NFS3 mount data
+ * Parse monolithic NFS2/NFS3 mount data
  * - fills in the mount root filehandle
  *
  * For option strings, user space handles the following behaviors:
@@ -1365,11 +1351,7 @@ int nfs_validate_text_mount_data(void *options,
 
 	nfs_set_port(sap, &cfg->nfs_server.port, port);
 
-	return nfs_parse_devname(dev_name,
-				   &cfg->nfs_server.hostname,
-				   max_namelen,
-				   &cfg->nfs_server.export_path,
-				   max_pathlen);
+	return nfs_parse_devname(cfg, dev_name, max_namelen, max_pathlen);
 
 #if !IS_ENABLED(CONFIG_NFS_V4)
 out_v4_not_compiled:
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 26c1fca31e6b..acf935a6438d 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -795,7 +795,7 @@ static int nfs_request_mount(struct nfs_sb_config *cfg,
 	/*
 	 * Construct the mount server's address.
 	 */
-	if (cfg->mount_server.address.ss_family == AF_UNSPEC) {
+	if (cfg->mount_server.address.sa_family == AF_UNSPEC) {
 		memcpy(request.sap, &cfg->nfs_server.address,
 		       cfg->nfs_server.addrlen);
 		cfg->mount_server.addrlen = cfg->nfs_server.addrlen;

[toc] | [prev] | [next] | [standalone]


#1641808 — [PATCH 17/21] NFS: Add a small buffer in nfs_sb_config to avoid string dup [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 17/21] NFS: Add a small buffer in nfs_sb_config to avoid string dup [ver #3]
Message-ID<tHq6C-1u3-11@gated-at.bofh.it>
In reply to#1641798
Add a small buffer in nfs_sb_config to avoid string duplication when
parsing numbers.  Also make the parsing function wrapper place the parsed
integer directly in the appropriate nfs_sb_config struct member.
---

 fs/nfs/internal.h |    2 +
 fs/nfs/mount.c    |   83 +++++++++++++++++++----------------------------------
 2 files changed, 32 insertions(+), 53 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index ed255241baa7..52242240933f 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -125,6 +125,8 @@ struct nfs_sb_config {
 
 	struct security_mnt_opts lsm_opts;
 	struct net		*net;
+
+	char			buf[32];	/* Parse buffer */
 };
 
 /* mount_clnt.c */
diff --git a/fs/nfs/mount.c b/fs/nfs/mount.c
index 54c38e0ed167..bebc589badbb 100644
--- a/fs/nfs/mount.c
+++ b/fs/nfs/mount.c
@@ -466,27 +466,22 @@ static int nfs_get_option_str(substring_t args[], char **option)
 	return !*option;
 }
 
-static int nfs_get_option_ul(substring_t args[], unsigned long *option)
+static int nfs_get_option_ui(struct nfs_sb_config *cfg,
+			     substring_t args[], unsigned int *option)
 {
-	int rc;
-	char *string;
-
-	string = match_strdup(args);
-	if (string == NULL)
-		return -ENOMEM;
-	rc = kstrtoul(string, 10, option);
-	kfree(string);
-
-	return rc;
+	match_strlcpy(cfg->buf, args, sizeof(cfg->buf));
+	return kstrtouint(cfg->buf, 10, option);
 }
 
-static int nfs_get_option_ul_bound(substring_t args[], unsigned long *option,
-		unsigned long l_bound, unsigned long u_bound)
+static int nfs_get_option_ui_bound(struct nfs_sb_config *cfg,
+				   substring_t args[], unsigned int *option,
+				   unsigned int l_bound, unsigned u_bound)
 {
 	int ret;
 
-	ret = nfs_get_option_ul(args, option);
-	if (ret != 0)
+	match_strlcpy(cfg->buf, args, sizeof(cfg->buf));
+	ret = kstrtouint(cfg->buf, 10, option);
+	if (ret < 0)
 		return ret;
 	if (*option < l_bound || *option > u_bound)
 		return -ERANGE;
@@ -499,7 +494,6 @@ static int nfs_get_option_ul_bound(substring_t args[], unsigned long *option,
 static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 {
 	substring_t args[MAX_OPT_ARGS];
-	unsigned long option;
 	char *string;
 	int rc, token;
 
@@ -507,7 +501,6 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 
 	token = match_token(p, nfs_mount_option_tokens, args);
 	switch (token) {
-
 		/*
 		 * boolean options:  foo/nofoo
 		 */
@@ -603,86 +596,70 @@ static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 		 * options that take numeric values
 		 */
 	case Opt_port:
-		if (nfs_get_option_ul(args, &option) ||
-		    option > USHRT_MAX)
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->nfs_server.port,
+					    0, USHRT_MAX))
 			goto out_invalid_value;
-		cfg->nfs_server.port = option;
 		break;
 	case Opt_rsize:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->rsize))
 			goto out_invalid_value;
-		cfg->rsize = option;
 		break;
 	case Opt_wsize:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->wsize))
 			goto out_invalid_value;
-		cfg->wsize = option;
 		break;
 	case Opt_bsize:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->bsize))
 			goto out_invalid_value;
-		cfg->bsize = option;
 		break;
 	case Opt_timeo:
-		if (nfs_get_option_ul_bound(args, &option, 1, INT_MAX))
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->timeo, 1, INT_MAX))
 			goto out_invalid_value;
-		cfg->timeo = option;
 		break;
 	case Opt_retrans:
-		if (nfs_get_option_ul_bound(args, &option, 0, INT_MAX))
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->retrans, 0, INT_MAX))
 			goto out_invalid_value;
-		cfg->retrans = option;
 		break;
 	case Opt_acregmin:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->acregmin))
 			goto out_invalid_value;
-		cfg->acregmin = option;
 		break;
 	case Opt_acregmax:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->acregmax))
 			goto out_invalid_value;
-		cfg->acregmax = option;
 		break;
 	case Opt_acdirmin:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->acdirmin))
 			goto out_invalid_value;
-		cfg->acdirmin = option;
 		break;
 	case Opt_acdirmax:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->acdirmax))
 			goto out_invalid_value;
-		cfg->acdirmax = option;
 		break;
 	case Opt_actimeo:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->acdirmax))
 			goto out_invalid_value;
 		cfg->acregmin = cfg->acregmax =
-			cfg->acdirmin = cfg->acdirmax = option;
+			cfg->acdirmin = cfg->acdirmax;
 		break;
 	case Opt_namelen:
-		if (nfs_get_option_ul(args, &option))
+		if (nfs_get_option_ui(cfg, args, &cfg->namlen))
 			goto out_invalid_value;
-		cfg->namlen = option;
 		break;
 	case Opt_mountport:
-		if (nfs_get_option_ul(args, &option) ||
-		    option > USHRT_MAX)
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->mount_server.port,
+					    0, USHRT_MAX))
 			goto out_invalid_value;
-		cfg->mount_server.port = option;
 		break;
 	case Opt_mountvers:
-		if (nfs_get_option_ul(args, &option) ||
-		    option < NFS_MNT_VERSION ||
-		    option > NFS_MNT3_VERSION)
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->mount_server.version,
+					    NFS_MNT_VERSION, NFS_MNT3_VERSION))
 			goto out_invalid_value;
-		cfg->mount_server.version = option;
 		break;
 	case Opt_minorversion:
-		if (nfs_get_option_ul(args, &option))
-			goto out_invalid_value;
-		if (option > NFS4_MAX_MINOR_VERSION)
+		if (nfs_get_option_ui_bound(cfg, args, &cfg->minorversion,
+					    0, NFS4_MAX_MINOR_VERSION))
 			goto out_invalid_value;
-		cfg->minorversion = option;
 		break;
 
 		/*

[toc] | [prev] | [next] | [standalone]


#1641813 — [PATCH 11/21] proc: Add superblock config support to procfs [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 11/21] proc: Add superblock config support to procfs [ver #3]
Message-ID<tHq6C-1u3-23@gated-at.bofh.it>
In reply to#1641798
Add superblock config support to procfs.

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

 fs/proc/inode.c    |    2 -
 fs/proc/internal.h |    2 -
 fs/proc/root.c     |  166 +++++++++++++++++++++++++++++++++-------------------
 3 files changed, 108 insertions(+), 62 deletions(-)

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index a4bf66af0ba9..304e5c36b7ba 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -118,7 +118,7 @@ const struct super_operations proc_sops = {
 	.drop_inode	= generic_delete_inode,
 	.evict_inode	= proc_evict_inode,
 	.statfs		= simple_statfs,
-	.remount_fs	= proc_remount,
+	.remount_fs_sc	= proc_remount,
 	.show_options	= proc_show_options,
 };
 
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index b681533f59dd..4546372c2d13 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -262,7 +262,7 @@ static inline void proc_tty_init(void) {}
 extern struct proc_dir_entry proc_root;
 
 extern void proc_self_init(void);
-extern int proc_remount(struct super_block *, int *, char *);
+extern int proc_remount(struct super_block *, struct sb_config *);
 
 /*
  * task_[no]mmu.c
diff --git a/fs/proc/root.c b/fs/proc/root.c
index ee1937b37370..da5757d1c518 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -24,9 +24,18 @@
 #include <linux/parser.h>
 #include <linux/cred.h>
 #include <linux/magic.h>
+#include <linux/slab.h>
 
 #include "internal.h"
 
+struct proc_sb_config {
+	struct sb_config	sc;
+	struct pid_namespace	*pid_ns;
+	unsigned long		mask;
+	int			hidepid;
+	int			gid;
+};
+
 enum {
 	Opt_gid, Opt_hidepid, Opt_err,
 };
@@ -37,56 +46,60 @@ static const match_table_t tokens = {
 	{Opt_err, NULL},
 };
 
-static int proc_parse_options(char *options, struct pid_namespace *pid)
+static int proc_parse_mount_option(struct sb_config *sc, char *p)
 {
-	char *p;
+	struct proc_sb_config *cfg = container_of(sc, struct proc_sb_config, sc);
 	substring_t args[MAX_OPT_ARGS];
-	int option;
-
-	if (!options)
-		return 1;
-
-	while ((p = strsep(&options, ",")) != NULL) {
-		int token;
-		if (!*p)
-			continue;
-
-		args[0].to = args[0].from = NULL;
-		token = match_token(p, tokens, args);
-		switch (token) {
-		case Opt_gid:
-			if (match_int(&args[0], &option))
-				return 0;
-			pid->pid_gid = make_kgid(current_user_ns(), option);
-			break;
-		case Opt_hidepid:
-			if (match_int(&args[0], &option))
-				return 0;
-			if (option < HIDEPID_OFF ||
-			    option > HIDEPID_INVISIBLE) {
-				pr_err("proc: hidepid value must be between 0 and 2.\n");
-				return 0;
-			}
-			pid->hide_pid = option;
-			break;
-		default:
-			pr_err("proc: unrecognized mount option \"%s\" "
-			       "or missing value\n", p);
-			return 0;
+	int token;
+
+	args[0].to = args[0].from = NULL;
+	token = match_token(p, tokens, args);
+	switch (token) {
+	case Opt_gid:
+		if (match_int(&args[0], &cfg->gid))
+			return sb_cfg_inval(sc, "procfs: Unparseable gid= argument");
+		break;
+
+	case Opt_hidepid:
+		if (match_int(&args[0], &cfg->hidepid))
+			return sb_cfg_inval(sc, "procfs: Unparseable hidepid= argument");
+		if (cfg->hidepid < HIDEPID_OFF ||
+		    cfg->hidepid > HIDEPID_INVISIBLE) {
+			pr_err("proc: hidepid value must be between 0 and 2.\n");
+			return sb_cfg_inval(sc, "procfs: Invalid hidepid= argument");
 		}
+		break;
+
+	default:
+		pr_err("proc: unrecognized mount option \"%s\" "
+		       "or missing value\n", p);
+		return sb_cfg_inval(sc, "procfs: Invalid mount option or missing value");
 	}
 
-	return 1;
+	cfg->mask |= 1 << token;
+	return 0;
+}
+
+static void proc_set_options(struct super_block *s,
+			     struct sb_config *sc,
+			     struct pid_namespace *pid_ns,
+			     struct user_namespace *user_ns)
+{
+	struct proc_sb_config *cfg = container_of(sc, struct proc_sb_config, sc);
+
+	if (cfg->mask & (1 << Opt_gid))
+		pid_ns->pid_gid = make_kgid(user_ns, cfg->gid);
+	if (cfg->mask & (1 << Opt_hidepid))
+		pid_ns->hide_pid = cfg->hidepid;
 }
 
-static int proc_fill_super(struct super_block *s, void *data, int silent)
+static int proc_fill_super(struct super_block *s, struct sb_config *sc)
 {
-	struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
+	struct pid_namespace *pid_ns = get_pid_ns(s->s_fs_info);
 	struct inode *root_inode;
 	int ret;
 
-	if (!proc_parse_options(data, ns))
-		return -EINVAL;
+	proc_set_options(s, sc, pid_ns, current_user_ns());
 
 	/* User space would break if executables or devices appear on proc */
 	s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
@@ -103,7 +116,7 @@ static int proc_fill_super(struct super_block *s, void *data, int silent)
 	 * top of it
 	 */
 	s->s_stack_depth = FILESYSTEM_MAX_STACK_DEPTH;
-	
+
 	pde_get(&proc_root);
 	root_inode = proc_get_inode(s, &proc_root);
 	if (!root_inode) {
@@ -124,27 +137,45 @@ static int proc_fill_super(struct super_block *s, void *data, int silent)
 	return proc_setup_thread_self(s);
 }
 
-int proc_remount(struct super_block *sb, int *flags, char *data)
+int proc_remount(struct super_block *sb, struct sb_config *sc)
 {
 	struct pid_namespace *pid = sb->s_fs_info;
 
 	sync_filesystem(sb);
-	return !proc_parse_options(data, pid);
+
+	if (sc)
+		proc_set_options(sb, sc, pid, current_user_ns());
+	return 0;
 }
 
-static struct dentry *proc_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
+static struct dentry *proc_mount(struct sb_config *sc)
 {
-	struct pid_namespace *ns;
+	struct proc_sb_config *cfg = container_of(sc, struct proc_sb_config, sc);
 
-	if (flags & MS_KERNMOUNT) {
-		ns = data;
-		data = NULL;
-	} else {
-		ns = task_active_pid_ns(current);
-	}
+	return mount_ns_sc(sc, proc_fill_super, cfg->pid_ns);
+}
 
-	return mount_ns(fs_type, flags, data, ns, ns->user_ns, proc_fill_super);
+static void proc_sb_config_free(struct sb_config *sc)
+{
+	struct proc_sb_config *cfg = container_of(sc, struct proc_sb_config, sc);
+
+	if (cfg->pid_ns)
+		put_pid_ns(cfg->pid_ns);
+}
+
+static const struct sb_config_operations proc_sb_config_ops = {
+	.free		= proc_sb_config_free,
+	.parse_option	= proc_parse_mount_option,
+	.mount		= proc_mount,
+};
+
+static int proc_init_sb_config(struct sb_config *sc, struct super_block *src_sb)
+{
+	struct proc_sb_config *cfg = container_of(sc, struct proc_sb_config, sc);
+
+	cfg->pid_ns = get_pid_ns(task_active_pid_ns(current));
+	cfg->sc.ops = &proc_sb_config_ops;
+	return 0;
 }
 
 static void proc_kill_sb(struct super_block *sb)
@@ -162,7 +193,8 @@ static void proc_kill_sb(struct super_block *sb)
 
 static struct file_system_type proc_fs_type = {
 	.name		= "proc",
-	.mount		= proc_mount,
+	.sb_config_size	= sizeof(struct proc_sb_config),
+	.init_sb_config	= proc_init_sb_config,
 	.kill_sb	= proc_kill_sb,
 	.fs_flags	= FS_USERNS_MOUNT,
 };
@@ -210,7 +242,7 @@ static struct dentry *proc_root_lookup(struct inode * dir, struct dentry * dentr
 {
 	if (!proc_pid_lookup(dir, dentry, flags))
 		return NULL;
-	
+
 	return proc_lookup(dir, dentry, flags);
 }
 
@@ -249,12 +281,12 @@ static const struct inode_operations proc_root_inode_operations = {
  * This is the root "inode" in the /proc tree..
  */
 struct proc_dir_entry proc_root = {
-	.low_ino	= PROC_ROOT_INO, 
-	.namelen	= 5, 
-	.mode		= S_IFDIR | S_IRUGO | S_IXUGO, 
-	.nlink		= 2, 
+	.low_ino	= PROC_ROOT_INO,
+	.namelen	= 5,
+	.mode		= S_IFDIR | S_IRUGO | S_IXUGO,
+	.nlink		= 2,
 	.count		= ATOMIC_INIT(1),
-	.proc_iops	= &proc_root_inode_operations, 
+	.proc_iops	= &proc_root_inode_operations,
 	.proc_fops	= &proc_root_operations,
 	.parent		= &proc_root,
 	.subdir		= RB_ROOT,
@@ -263,9 +295,23 @@ struct proc_dir_entry proc_root = {
 
 int pid_ns_prepare_proc(struct pid_namespace *ns)
 {
+	struct proc_sb_config *cfg;
+	struct sb_config *sc;
 	struct vfsmount *mnt;
 
-	mnt = kern_mount_data(&proc_fs_type, ns);
+	sc = __vfs_new_sb_config(&proc_fs_type, NULL, 0, SB_CONFIG_FOR_NEW);
+	if (IS_ERR(sc))
+		return PTR_ERR(sc);
+
+	cfg = container_of(sc, struct proc_sb_config, sc);
+	if (cfg->pid_ns != ns) {
+		put_pid_ns(cfg->pid_ns);
+		get_pid_ns(ns);
+		cfg->pid_ns = ns;
+	}
+
+	mnt = kern_mount_data_sc(sc);
+	put_sb_config(sc);
 	if (IS_ERR(mnt))
 		return PTR_ERR(mnt);
 

[toc] | [prev] | [next] | [standalone]


#1641815 — [PATCH 15/21] NFS: Split nfs_parse_mount_options() [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 15/21] NFS: Split nfs_parse_mount_options() [ver #3]
Message-ID<tHq6D-1u3-31@gated-at.bofh.it>
In reply to#1641798
Split nfs_parse_mount_options() to move the prologue, list-splitting and
epilogue into one function and the per-option processing into another.
---

 fs/nfs/internal.h |    3 +
 fs/nfs/mount.c    |  143 ++++++++++++++++++++++++++++++-----------------------
 2 files changed, 83 insertions(+), 63 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index af6ca28da48d..ed255241baa7 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -100,7 +100,10 @@ struct nfs_sb_config {
 	unsigned int		version;
 	unsigned int		minorversion;
 	char			*fscache_uniq;
+	unsigned short		protofamily;
+	unsigned short		mountfamily;
 	bool			need_mount;
+	bool			sloppy;
 
 	struct {
 		struct sockaddr_storage	address;
diff --git a/fs/nfs/mount.c b/fs/nfs/mount.c
index ef9683deb2a8..26153b471e48 100644
--- a/fs/nfs/mount.c
+++ b/fs/nfs/mount.c
@@ -494,46 +494,18 @@ static int nfs_get_option_ul_bound(substring_t args[], unsigned long *option,
 }
 
 /*
- * Error-check and convert a string of mount options from user space into
- * a data structure.  The whole mount string is processed; bad options are
- * skipped as they are encountered.  If there were no errors, return 1;
- * otherwise return 0 (zero).
+ * Parse a single mount option in "key[=val]" form.
  */
-int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
+static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 {
-	char *p, *string, *secdata;
-	int rc, sloppy = 0, invalid_option = 0;
-	unsigned short protofamily = AF_UNSPEC;
-	unsigned short mountfamily = AF_UNSPEC;
-
-	if (!raw) {
-		dfprintk(MOUNT, "NFS: mount options string was NULL.\n");
-		return 1;
-	}
-	dfprintk(MOUNT, "NFS: nfs mount opts='%s'\n", raw);
-
-	secdata = alloc_secdata();
-	if (!secdata)
-		goto out_nomem;
-
-	rc = security_sb_copy_data(raw, secdata);
-	if (rc)
-		goto out_security_failure;
-
-	rc = security_sb_parse_opts_str(secdata, &cfg->lsm_opts);
-	if (rc)
-		goto out_security_failure;
-
-	free_secdata(secdata);
+	char *string;
+	int rc;
 
-	while ((p = strsep(&raw, ",")) != NULL) {
+	{
 		substring_t args[MAX_OPT_ARGS];
 		unsigned long option;
 		int token;
 
-		if (!*p)
-			continue;
-
 		dfprintk(MOUNT, "NFS:   parsing nfs mount option '%s'\n", p);
 
 		token = match_token(p, nfs_mount_option_tokens, args);
@@ -737,7 +709,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 			if (!rc) {
 				dfprintk(MOUNT, "NFS:   unrecognized "
 						"security flavor\n");
-				return 0;
+				return -EINVAL;
 			}
 			break;
 		case Opt_proto:
@@ -747,22 +719,22 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 			token = match_token(string,
 					    nfs_xprt_protocol_tokens, args);
 
-			protofamily = AF_INET;
+			cfg->protofamily = AF_INET;
 			switch (token) {
 			case Opt_xprt_udp6:
-				protofamily = AF_INET6;
+				cfg->protofamily = AF_INET6;
 			case Opt_xprt_udp:
 				cfg->flags &= ~NFS_MOUNT_TCP;
 				cfg->nfs_server.protocol = XPRT_TRANSPORT_UDP;
 				break;
 			case Opt_xprt_tcp6:
-				protofamily = AF_INET6;
+				cfg->protofamily = AF_INET6;
 			case Opt_xprt_tcp:
 				cfg->flags |= NFS_MOUNT_TCP;
 				cfg->nfs_server.protocol = XPRT_TRANSPORT_TCP;
 				break;
 			case Opt_xprt_rdma6:
-				protofamily = AF_INET6;
+				cfg->protofamily = AF_INET6;
 			case Opt_xprt_rdma:
 				/* vector side protocols to TCP */
 				cfg->flags |= NFS_MOUNT_TCP;
@@ -773,7 +745,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 				dfprintk(MOUNT, "NFS:   unrecognized "
 						"transport protocol\n");
 				kfree(string);
-				return 0;
+				return -EINVAL;
 			}
 			kfree(string);
 			break;
@@ -785,15 +757,15 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 					    nfs_xprt_protocol_tokens, args);
 			kfree(string);
 
-			mountfamily = AF_INET;
+			cfg->mountfamily = AF_INET;
 			switch (token) {
 			case Opt_xprt_udp6:
-				mountfamily = AF_INET6;
+				cfg->mountfamily = AF_INET6;
 			case Opt_xprt_udp:
 				cfg->mount_server.protocol = XPRT_TRANSPORT_UDP;
 				break;
 			case Opt_xprt_tcp6:
-				mountfamily = AF_INET6;
+				cfg->mountfamily = AF_INET6;
 			case Opt_xprt_tcp:
 				cfg->mount_server.protocol = XPRT_TRANSPORT_TCP;
 				break;
@@ -801,7 +773,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 			default:
 				dfprintk(MOUNT, "NFS:   unrecognized "
 						"transport protocol\n");
-				return 0;
+				return -EINVAL;
 			}
 			break;
 		case Opt_addr:
@@ -860,7 +832,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 				default:
 					dfprintk(MOUNT, "NFS:   invalid "
 							"lookupcache argument\n");
-					return 0;
+					return -EINVAL;
 			};
 			break;
 		case Opt_fscache_uniq:
@@ -893,7 +865,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 			default:
 				dfprintk(MOUNT, "NFS:	invalid	"
 						"local_lock argument\n");
-				return 0;
+				return -EINVAL;
 			};
 			break;
 
@@ -901,7 +873,7 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 		 * Special options
 		 */
 		case Opt_sloppy:
-			sloppy = 1;
+			cfg->sloppy = 1;
 			dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
 			break;
 		case Opt_userspace:
@@ -911,12 +883,63 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 			break;
 
 		default:
-			invalid_option = 1;
 			dfprintk(MOUNT, "NFS:   unrecognized mount option "
 					"'%s'\n", p);
+			return -EINVAL;
 		}
 	}
 
+	return 0;
+
+out_invalid_address:
+	printk(KERN_INFO "NFS: bad IP address specified: %s\n", p);
+	return -EINVAL;
+out_invalid_value:
+	printk(KERN_INFO "NFS: bad mount option value specified: %s\n", p);
+	return -EINVAL;
+out_nomem:
+	printk(KERN_INFO "NFS: not enough memory to parse option\n");
+	return -ENOMEM;
+}
+
+/*
+ * Error-check and convert a string of mount options from user space into
+ * a data structure.  The whole mount string is processed; bad options are
+ * skipped as they are encountered.  If there were no errors, return 1;
+ * otherwise return 0 (zero).
+ */
+int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
+{
+	char *p, *secdata;
+	int rc, sloppy = 0, invalid_option = 0;
+
+	if (!raw) {
+		dfprintk(MOUNT, "NFS: mount options string was NULL.\n");
+		return 1;
+	}
+	dfprintk(MOUNT, "NFS: nfs mount opts='%s'\n", raw);
+
+	secdata = alloc_secdata();
+	if (!secdata)
+		goto out_nomem;
+
+	rc = security_sb_copy_data(raw, secdata);
+	if (rc)
+		goto out_security_failure;
+
+	rc = security_sb_parse_opts_str(secdata, &cfg->lsm_opts);
+	if (rc)
+		goto out_security_failure;
+
+	free_secdata(secdata);
+
+	while ((p = strsep(&raw, ",")) != NULL) {
+		if (!*p)
+			continue;
+		if (nfs_sb_config_parse_option(cfg, p) < 0)
+			invalid_option = true;
+	}
+
 	if (!sloppy && invalid_option)
 		return 0;
 
@@ -931,22 +954,26 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 	 * verify that any proto=/mountproto= options match the address
 	 * families in the addr=/mountaddr= options.
 	 */
-	if (protofamily != AF_UNSPEC &&
-	    protofamily != cfg->nfs_server.address.ss_family)
+	if (cfg->protofamily != AF_UNSPEC &&
+	    cfg->protofamily != cfg->nfs_server.address.ss_family)
 		goto out_proto_mismatch;
 
-	if (mountfamily != AF_UNSPEC) {
+	if (cfg->mountfamily != AF_UNSPEC) {
 		if (cfg->mount_server.addrlen) {
-			if (mountfamily != cfg->mount_server.address.ss_family)
+			if (cfg->mountfamily != cfg->mount_server.address.ss_family)
 				goto out_mountproto_mismatch;
 		} else {
-			if (mountfamily != cfg->nfs_server.address.ss_family)
+			if (cfg->mountfamily != cfg->nfs_server.address.ss_family)
 				goto out_mountproto_mismatch;
 		}
 	}
 
 	return 1;
 
+out_minorversion_mismatch:
+	printk(KERN_INFO "NFS: mount option vers=%u does not support "
+			 "minorversion=%u\n", cfg->version, cfg->minorversion);
+	return 0;
 out_mountproto_mismatch:
 	printk(KERN_INFO "NFS: mount server address does not match mountproto= "
 			 "option\n");
@@ -954,20 +981,10 @@ int nfs_parse_mount_options(char *raw, struct nfs_sb_config *cfg)
 out_proto_mismatch:
 	printk(KERN_INFO "NFS: server address does not match proto= option\n");
 	return 0;
-out_invalid_address:
-	printk(KERN_INFO "NFS: bad IP address specified: %s\n", p);
-	return 0;
-out_invalid_value:
-	printk(KERN_INFO "NFS: bad mount option value specified: %s\n", p);
-	return 0;
-out_minorversion_mismatch:
-	printk(KERN_INFO "NFS: mount option vers=%u does not support "
-			 "minorversion=%u\n", cfg->version, cfg->minorversion);
-	return 0;
 out_migration_misuse:
 	printk(KERN_INFO
 		"NFS: 'migration' not supported for this NFS version\n");
-	return 0;
+	return -EINVAL;
 out_nomem:
 	printk(KERN_INFO "NFS: not enough memory to parse option\n");
 	return 0;

[toc] | [prev] | [next] | [standalone]


#1641816 — [PATCH 20/21] Support legacy filesystems [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 20/21] Support legacy filesystems [ver #3]
Message-ID<tHq6D-1u3-33@gated-at.bofh.it>
In reply to#1641798
Support legacy filesystems by creating a set of legacy sb_config operations
that builds up a list of mount options and then invokes fs_type->mount()
within the sb_config mount operation.

All filesystems can then be accessed using sb_config and the
fs_type->mount() call is _only_ used from within legacy_mount().  This
allows some simplification to take place in the core mount code.
---

 fs/namespace.c |   36 ++-----------
 fs/sb_config.c |  160 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 161 insertions(+), 35 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index cbac401e12a1..ce892343d1de 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2574,7 +2574,6 @@ static int do_new_mount(struct path *mountpoint, const char *fstype, int flags,
 			int mnt_flags, const char *name, void *data)
 {
 	struct sb_config *sc;
-	struct vfsmount *mnt;
 	int err;
 
 	if (!fstype)
@@ -2590,40 +2589,17 @@ static int do_new_mount(struct path *mountpoint, const char *fstype, int flags,
 	if (!sc->device)
 		goto err_sc;
 
-	if (sc->ops) {
-		err = parse_monolithic_mount_data(sc, data);
-		if (err < 0)
-			goto err_sc;
-
-		err = do_new_mount_sc(sc, mountpoint, mnt_flags);
-		if (err)
-			goto err_sc;
-
-	} else {
-		mnt = vfs_kern_mount(sc->fs_type, flags, name, data);
-		if (!IS_ERR(mnt) && (sc->fs_type->fs_flags & FS_HAS_SUBTYPE) &&
-		    !mnt->mnt_sb->s_subtype)
-			mnt = fs_set_subtype(mnt, fstype);
-
-		if (IS_ERR(mnt)) {
-			err = PTR_ERR(mnt);
-			goto err_sc;
-		}
-
-		err = -EPERM;
-		if (mount_too_revealing(mnt, &mnt_flags))
-			goto err_mnt;
+	err = parse_monolithic_mount_data(sc, data);
+	if (err < 0)
+		goto err_sc;
 
-		err = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
-		if (err)
-			goto err_mnt;
-	}
+	err = do_new_mount_sc(sc, mountpoint, mnt_flags);
+	if (err)
+		goto err_sc;
 
 	put_sb_config(sc);
 	return 0;
 
-err_mnt:
-	mntput(mnt);
 err_sc:
 	if (sc->error_msg)
 		pr_info("Mount failed: %s\n", sc->error_msg);
diff --git a/fs/sb_config.c b/fs/sb_config.c
index 9c45e269b3cc..62e309cd62ef 100644
--- a/fs/sb_config.c
+++ b/fs/sb_config.c
@@ -25,6 +25,15 @@
 #include <net/net_namespace.h>
 #include "mount.h"
 
+struct legacy_sb_config {
+	struct sb_config	sc;
+	char			*legacy_data;	/* Data page for legacy filesystems */
+	char			*secdata;
+	unsigned int		data_usage;
+};
+
+static const struct sb_config_operations legacy_sb_config_ops;
+
 static const match_table_t common_set_mount_options = {
 	{ MS_DIRSYNC,		"dirsync" },
 	{ MS_I_VERSION,		"iversion" },
@@ -186,13 +195,15 @@ struct sb_config *__vfs_new_sb_config(struct file_system_type *fs_type,
 				      enum sb_config_purpose purpose)
 {
 	struct sb_config *sc;
+	size_t sc_size = fs_type->sb_config_size;
 	int ret;
 
-	BUG_ON(fs_type->init_sb_config &&
-	       fs_type->sb_config_size < sizeof(*sc));
+	BUG_ON(fs_type->init_sb_config && sc_size < sizeof(*sc));
+
+	if (!fs_type->init_sb_config)
+		sc_size = sizeof(struct legacy_sb_config);
 
-	sc = kzalloc(max_t(size_t, fs_type->sb_config_size, sizeof(*sc)),
-		     GFP_KERNEL);
+	sc = kzalloc(sc_size, GFP_KERNEL);
 	if (!sc)
 		return ERR_PTR(-ENOMEM);
 
@@ -208,9 +219,11 @@ struct sb_config *__vfs_new_sb_config(struct file_system_type *fs_type,
 		ret = sc->fs_type->init_sb_config(sc, src_sb);
 		if (ret < 0)
 			goto err_sc;
+	} else {
+		sc->ops = &legacy_sb_config_ops;
 	}
 
-	/* Do the security check last because ->fsopen may change the
+	/* Do the security check last because ->init_sb_config may change the
 	 * namespace subscriptions.
 	 */
 	ret = security_sb_config_alloc(sc, src_sb);
@@ -272,11 +285,16 @@ struct sb_config *vfs_sb_reconfig(struct vfsmount *mnt,
 struct sb_config *vfs_dup_sb_config(struct sb_config *src_sc)
 {
 	struct sb_config *sc;
+	size_t sc_size;
 	int ret;
 
 	if (!src_sc->ops->dup)
 		return ERR_PTR(-ENOTSUPP);
 
+	sc_size = src_sc->fs_type->sb_config_size;
+	if (!src_sc->fs_type->init_sb_config)
+		sc_size = sizeof(struct legacy_sb_config);
+
 	sc = kmemdup(src_sc, src_sc->fs_type->sb_config_size, GFP_KERNEL);
 	if (!sc)
 		return ERR_PTR(-ENOMEM);
@@ -324,3 +342,135 @@ void put_sb_config(struct sb_config *sc)
 	kfree(sc);
 }
 EXPORT_SYMBOL(put_sb_config);
+
+/*
+ * Free the config for a filesystem that doesn't support sb_config.
+ */
+static void legacy_sb_config_free(struct sb_config *sc)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+
+	free_secdata(cfg->secdata);
+	kfree(cfg->legacy_data);
+}
+
+/*
+ * Duplicate a legacy config.
+ */
+static int legacy_sb_config_dup(struct sb_config *sc, struct sb_config *src_sc)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+	struct legacy_sb_config *src_cfg = container_of(src_sc, struct legacy_sb_config, sc);
+
+	cfg->legacy_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!cfg->legacy_data)
+		return -ENOMEM;
+	memcpy(cfg->legacy_data, src_cfg->legacy_data, sizeof(PAGE_SIZE));
+	return 0;
+}
+
+/*
+ * Add an option to a legacy config.  We build up a comma-separated list of
+ * options.
+ */
+static int legacy_parse_option(struct sb_config *sc, char *p)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+	unsigned int usage = cfg->data_usage;
+	size_t len = strlen(p);
+
+	if (len > PAGE_SIZE - 2 - usage)
+		return sb_cfg_inval(sc, "VFS: Insufficient data buffer space");
+	if (memchr(p, ',', len) != NULL)
+		return sb_cfg_inval(sc, "VFS: Options cannot contain commas");
+	if (!cfg->legacy_data) {
+		cfg->legacy_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!cfg->legacy_data)
+			return -ENOMEM;
+	}
+
+	cfg->legacy_data[usage++] = ',';
+	memcpy(cfg->legacy_data + usage, p, len);
+	usage += len;
+	cfg->legacy_data[usage] = '\0';
+	cfg->data_usage = usage;
+	return 0;
+}
+
+/*
+ * Add monolithic mount data.
+ */
+static int legacy_monolithic_mount_data(struct sb_config *sc, void *data)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+
+	if (cfg->data_usage != 0)
+		return sb_cfg_inval(sc, "VFS: Can't mix monolithic and individual options");
+	if (!data)
+		return 0;
+	if (!cfg->legacy_data) {
+		cfg->legacy_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
+		if (!cfg->legacy_data)
+			return -ENOMEM;
+	}
+
+	memcpy(cfg->legacy_data, data, PAGE_SIZE);
+	cfg->data_usage = PAGE_SIZE;
+	return 0;
+}
+
+/*
+ * Use the legacy mount validation step to strip out and process security
+ * config options.
+ */
+static int legacy_validate(struct sb_config *sc)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+
+	if (!cfg->legacy_data || cfg->sc.fs_type->fs_flags & FS_BINARY_MOUNTDATA)
+		return 0;
+
+	cfg->secdata = alloc_secdata();
+	if (!cfg->secdata)
+		return -ENOMEM;
+
+	return security_sb_copy_data(cfg->legacy_data, cfg->secdata);
+}
+
+/*
+ * Perform a legacy mount.
+ */
+static struct dentry *legacy_mount(struct sb_config *sc)
+{
+	struct legacy_sb_config *cfg = container_of(sc, struct legacy_sb_config, sc);
+	struct super_block *sb;
+	struct dentry *root;
+	int ret;
+
+	root = cfg->sc.fs_type->mount(cfg->sc.fs_type, cfg->sc.ms_flags,
+				     cfg->sc.device, cfg->legacy_data);
+	if (IS_ERR(root))
+		return ERR_CAST(root);
+
+	sb = root->d_sb;
+	BUG_ON(!sb);
+	ret = security_sb_kern_mount(sb, cfg->sc.ms_flags, cfg->secdata);
+	if (ret < 0)
+		goto err_sb;
+
+	return root;
+
+err_sb:
+	dput(root);
+	deactivate_locked_super(sb);
+	return ERR_PTR(ret);
+}
+
+static const struct sb_config_operations legacy_sb_config_ops = {
+	.free			= legacy_sb_config_free,
+	.dup			= legacy_sb_config_dup,
+	.parse_option		= legacy_parse_option,
+	.monolithic_mount_data	= legacy_monolithic_mount_data,
+	.validate		= legacy_validate,
+	.mount			= legacy_mount,
+};

[toc] | [prev] | [next] | [standalone]


#1641817 — [PATCH 13/21] NFS: Constify mount argument match tables [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 13/21] NFS: Constify mount argument match tables [ver #3]
Message-ID<tHq6C-1u3-25@gated-at.bofh.it>
In reply to#1641798
---

 fs/nfs/mount.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/mount.c b/fs/nfs/mount.c
index eb77d9af6c35..9b5b89acf97a 100644
--- a/fs/nfs/mount.c
+++ b/fs/nfs/mount.c
@@ -197,7 +197,7 @@ enum {
 	Opt_lookupcache_err
 };
 
-static match_table_t nfs_lookupcache_tokens = {
+static const match_table_t nfs_lookupcache_tokens = {
 	{ Opt_lookupcache_all, "all" },
 	{ Opt_lookupcache_positive, "pos" },
 	{ Opt_lookupcache_positive, "positive" },
@@ -213,7 +213,7 @@ enum {
 	Opt_local_lock_err
 };
 
-static match_table_t nfs_local_lock_tokens = {
+static const match_table_t nfs_local_lock_tokens = {
 	{ Opt_local_lock_all, "all" },
 	{ Opt_local_lock_flock, "flock" },
 	{ Opt_local_lock_posix, "posix" },
@@ -229,7 +229,7 @@ enum {
 	Opt_vers_err
 };
 
-static match_table_t nfs_vers_tokens = {
+static const match_table_t nfs_vers_tokens = {
 	{ Opt_vers_2, "2" },
 	{ Opt_vers_3, "3" },
 	{ Opt_vers_4, "4" },

[toc] | [prev] | [next] | [standalone]


#1641818 — [PATCH 16/21] NFS: Deindent nfs_sb_config_parse_option() [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-15 17:30 +0200
Subject[PATCH 16/21] NFS: Deindent nfs_sb_config_parse_option() [ver #3]
Message-ID<tHq6D-1u3-29@gated-at.bofh.it>
In reply to#1641798
Deindent nfs_sb_config_parse_option().
---

 fs/nfs/mount.c |  706 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 351 insertions(+), 355 deletions(-)

diff --git a/fs/nfs/mount.c b/fs/nfs/mount.c
index 26153b471e48..54c38e0ed167 100644
--- a/fs/nfs/mount.c
+++ b/fs/nfs/mount.c
@@ -498,395 +498,391 @@ static int nfs_get_option_ul_bound(substring_t args[], unsigned long *option,
  */
 static int nfs_sb_config_parse_option(struct nfs_sb_config *cfg, char *p)
 {
+	substring_t args[MAX_OPT_ARGS];
+	unsigned long option;
 	char *string;
-	int rc;
-
-	{
-		substring_t args[MAX_OPT_ARGS];
-		unsigned long option;
-		int token;
+	int rc, token;
 
-		dfprintk(MOUNT, "NFS:   parsing nfs mount option '%s'\n", p);
+	dfprintk(MOUNT, "NFS:   parsing nfs mount option '%s'\n", p);
 
-		token = match_token(p, nfs_mount_option_tokens, args);
-		switch (token) {
+	token = match_token(p, nfs_mount_option_tokens, args);
+	switch (token) {
 
 		/*
 		 * boolean options:  foo/nofoo
 		 */
-		case Opt_soft:
-			cfg->flags |= NFS_MOUNT_SOFT;
-			break;
-		case Opt_hard:
-			cfg->flags &= ~NFS_MOUNT_SOFT;
-			break;
-		case Opt_posix:
-			cfg->flags |= NFS_MOUNT_POSIX;
-			break;
-		case Opt_noposix:
-			cfg->flags &= ~NFS_MOUNT_POSIX;
-			break;
-		case Opt_cto:
-			cfg->flags &= ~NFS_MOUNT_NOCTO;
-			break;
-		case Opt_nocto:
-			cfg->flags |= NFS_MOUNT_NOCTO;
-			break;
-		case Opt_ac:
-			cfg->flags &= ~NFS_MOUNT_NOAC;
-			break;
-		case Opt_noac:
-			cfg->flags |= NFS_MOUNT_NOAC;
-			break;
-		case Opt_lock:
-			cfg->flags &= ~NFS_MOUNT_NONLM;
-			cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
-					NFS_MOUNT_LOCAL_FCNTL);
-			break;
-		case Opt_nolock:
-			cfg->flags |= NFS_MOUNT_NONLM;
-			cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK |
-				       NFS_MOUNT_LOCAL_FCNTL);
-			break;
-		case Opt_udp:
-			cfg->flags &= ~NFS_MOUNT_TCP;
-			cfg->nfs_server.protocol = XPRT_TRANSPORT_UDP;
-			break;
-		case Opt_tcp:
-			cfg->flags |= NFS_MOUNT_TCP;
-			cfg->nfs_server.protocol = XPRT_TRANSPORT_TCP;
-			break;
-		case Opt_rdma:
-			cfg->flags |= NFS_MOUNT_TCP; /* for side protocols */
-			cfg->nfs_server.protocol = XPRT_TRANSPORT_RDMA;
-			xprt_load_transport(p);
-			break;
-		case Opt_acl:
-			cfg->flags &= ~NFS_MOUNT_NOACL;
-			break;
-		case Opt_noacl:
-			cfg->flags |= NFS_MOUNT_NOACL;
-			break;
-		case Opt_rdirplus:
-			cfg->flags &= ~NFS_MOUNT_NORDIRPLUS;
-			break;
-		case Opt_nordirplus:
-			cfg->flags |= NFS_MOUNT_NORDIRPLUS;
-			break;
-		case Opt_sharecache:
-			cfg->flags &= ~NFS_MOUNT_UNSHARED;
-			break;
-		case Opt_nosharecache:
-			cfg->flags |= NFS_MOUNT_UNSHARED;
-			break;
-		case Opt_resvport:
-			cfg->flags &= ~NFS_MOUNT_NORESVPORT;
-			break;
-		case Opt_noresvport:
-			cfg->flags |= NFS_MOUNT_NORESVPORT;
-			break;
-		case Opt_fscache:
-			cfg->options |= NFS_OPTION_FSCACHE;
-			kfree(cfg->fscache_uniq);
-			cfg->fscache_uniq = NULL;
-			break;
-		case Opt_nofscache:
-			cfg->options &= ~NFS_OPTION_FSCACHE;
-			kfree(cfg->fscache_uniq);
-			cfg->fscache_uniq = NULL;
-			break;
-		case Opt_migration:
-			cfg->options |= NFS_OPTION_MIGRATION;
-			break;
-		case Opt_nomigration:
-			cfg->options &= NFS_OPTION_MIGRATION;
-			break;
+	case Opt_soft:
+		cfg->flags |= NFS_MOUNT_SOFT;
+		break;
+	case Opt_hard:
+		cfg->flags &= ~NFS_MOUNT_SOFT;
+		break;
+	case Opt_posix:
+		cfg->flags |= NFS_MOUNT_POSIX;
+		break;
+	case Opt_noposix:
+		cfg->flags &= ~NFS_MOUNT_POSIX;
+		break;
+	case Opt_cto:
+		cfg->flags &= ~NFS_MOUNT_NOCTO;
+		break;
+	case Opt_nocto:
+		cfg->flags |= NFS_MOUNT_NOCTO;
+		break;
+	case Opt_ac:
+		cfg->flags &= ~NFS_MOUNT_NOAC;
+		break;
+	case Opt_noac:
+		cfg->flags |= NFS_MOUNT_NOAC;
+		break;
+	case Opt_lock:
+		cfg->flags &= ~NFS_MOUNT_NONLM;
+		cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+				NFS_MOUNT_LOCAL_FCNTL);
+		break;
+	case Opt_nolock:
+		cfg->flags |= NFS_MOUNT_NONLM;
+		cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+			       NFS_MOUNT_LOCAL_FCNTL);
+		break;
+	case Opt_udp:
+		cfg->flags &= ~NFS_MOUNT_TCP;
+		cfg->nfs_server.protocol = XPRT_TRANSPORT_UDP;
+		break;
+	case Opt_tcp:
+		cfg->flags |= NFS_MOUNT_TCP;
+		cfg->nfs_server.protocol = XPRT_TRANSPORT_TCP;
+		break;
+	case Opt_rdma:
+		cfg->flags |= NFS_MOUNT_TCP; /* for side protocols */
+		cfg->nfs_server.protocol = XPRT_TRANSPORT_RDMA;
+		xprt_load_transport(p);
+		break;
+	case Opt_acl:
+		cfg->flags &= ~NFS_MOUNT_NOACL;
+		break;
+	case Opt_noacl:
+		cfg->flags |= NFS_MOUNT_NOACL;
+		break;
+	case Opt_rdirplus:
+		cfg->flags &= ~NFS_MOUNT_NORDIRPLUS;
+		break;
+	case Opt_nordirplus:
+		cfg->flags |= NFS_MOUNT_NORDIRPLUS;
+		break;
+	case Opt_sharecache:
+		cfg->flags &= ~NFS_MOUNT_UNSHARED;
+		break;
+	case Opt_nosharecache:
+		cfg->flags |= NFS_MOUNT_UNSHARED;
+		break;
+	case Opt_resvport:
+		cfg->flags &= ~NFS_MOUNT_NORESVPORT;
+		break;
+	case Opt_noresvport:
+		cfg->flags |= NFS_MOUNT_NORESVPORT;
+		break;
+	case Opt_fscache:
+		cfg->options |= NFS_OPTION_FSCACHE;
+		kfree(cfg->fscache_uniq);
+		cfg->fscache_uniq = NULL;
+		break;
+	case Opt_nofscache:
+		cfg->options &= ~NFS_OPTION_FSCACHE;
+		kfree(cfg->fscache_uniq);
+		cfg->fscache_uniq = NULL;
+		break;
+	case Opt_migration:
+		cfg->options |= NFS_OPTION_MIGRATION;
+		break;
+	case Opt_nomigration:
+		cfg->options &= NFS_OPTION_MIGRATION;
+		break;
 
 		/*
 		 * options that take numeric values
 		 */
-		case Opt_port:
-			if (nfs_get_option_ul(args, &option) ||
-			    option > USHRT_MAX)
-				goto out_invalid_value;
-			cfg->nfs_server.port = option;
-			break;
-		case Opt_rsize:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->rsize = option;
-			break;
-		case Opt_wsize:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->wsize = option;
-			break;
-		case Opt_bsize:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->bsize = option;
-			break;
-		case Opt_timeo:
-			if (nfs_get_option_ul_bound(args, &option, 1, INT_MAX))
-				goto out_invalid_value;
-			cfg->timeo = option;
-			break;
-		case Opt_retrans:
-			if (nfs_get_option_ul_bound(args, &option, 0, INT_MAX))
-				goto out_invalid_value;
-			cfg->retrans = option;
-			break;
-		case Opt_acregmin:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->acregmin = option;
-			break;
-		case Opt_acregmax:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->acregmax = option;
-			break;
-		case Opt_acdirmin:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->acdirmin = option;
-			break;
-		case Opt_acdirmax:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->acdirmax = option;
-			break;
-		case Opt_actimeo:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->acregmin = cfg->acregmax =
+	case Opt_port:
+		if (nfs_get_option_ul(args, &option) ||
+		    option > USHRT_MAX)
+			goto out_invalid_value;
+		cfg->nfs_server.port = option;
+		break;
+	case Opt_rsize:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->rsize = option;
+		break;
+	case Opt_wsize:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->wsize = option;
+		break;
+	case Opt_bsize:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->bsize = option;
+		break;
+	case Opt_timeo:
+		if (nfs_get_option_ul_bound(args, &option, 1, INT_MAX))
+			goto out_invalid_value;
+		cfg->timeo = option;
+		break;
+	case Opt_retrans:
+		if (nfs_get_option_ul_bound(args, &option, 0, INT_MAX))
+			goto out_invalid_value;
+		cfg->retrans = option;
+		break;
+	case Opt_acregmin:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->acregmin = option;
+		break;
+	case Opt_acregmax:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->acregmax = option;
+		break;
+	case Opt_acdirmin:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->acdirmin = option;
+		break;
+	case Opt_acdirmax:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->acdirmax = option;
+		break;
+	case Opt_actimeo:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->acregmin = cfg->acregmax =
 			cfg->acdirmin = cfg->acdirmax = option;
-			break;
-		case Opt_namelen:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			cfg->namlen = option;
-			break;
-		case Opt_mountport:
-			if (nfs_get_option_ul(args, &option) ||
-			    option > USHRT_MAX)
-				goto out_invalid_value;
-			cfg->mount_server.port = option;
-			break;
-		case Opt_mountvers:
-			if (nfs_get_option_ul(args, &option) ||
-			    option < NFS_MNT_VERSION ||
-			    option > NFS_MNT3_VERSION)
-				goto out_invalid_value;
-			cfg->mount_server.version = option;
-			break;
-		case Opt_minorversion:
-			if (nfs_get_option_ul(args, &option))
-				goto out_invalid_value;
-			if (option > NFS4_MAX_MINOR_VERSION)
-				goto out_invalid_value;
-			cfg->minorversion = option;
-			break;
+		break;
+	case Opt_namelen:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		cfg->namlen = option;
+		break;
+	case Opt_mountport:
+		if (nfs_get_option_ul(args, &option) ||
+		    option > USHRT_MAX)
+			goto out_invalid_value;
+		cfg->mount_server.port = option;
+		break;
+	case Opt_mountvers:
+		if (nfs_get_option_ul(args, &option) ||
+		    option < NFS_MNT_VERSION ||
+		    option > NFS_MNT3_VERSION)
+			goto out_invalid_value;
+		cfg->mount_server.version = option;
+		break;
+	case Opt_minorversion:
+		if (nfs_get_option_ul(args, &option))
+			goto out_invalid_value;
+		if (option > NFS4_MAX_MINOR_VERSION)
+			goto out_invalid_value;
+		cfg->minorversion = option;
+		break;
 
 		/*
 		 * options that take text values
 		 */
-		case Opt_nfsvers:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			rc = nfs_parse_version_string(string, cfg, args);
-			kfree(string);
-			if (!rc)
-				goto out_invalid_value;
+	case Opt_nfsvers:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		rc = nfs_parse_version_string(string, cfg, args);
+		kfree(string);
+		if (!rc)
+			goto out_invalid_value;
+		break;
+	case Opt_sec:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		rc = nfs_parse_security_flavors(string, cfg);
+		kfree(string);
+		if (!rc) {
+			dfprintk(MOUNT, "NFS:   unrecognized "
+				 "security flavor\n");
+			return -EINVAL;
+		}
+		break;
+	case Opt_proto:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		token = match_token(string,
+				    nfs_xprt_protocol_tokens, args);
+
+		cfg->protofamily = AF_INET;
+		switch (token) {
+		case Opt_xprt_udp6:
+			cfg->protofamily = AF_INET6;
+		case Opt_xprt_udp:
+			cfg->flags &= ~NFS_MOUNT_TCP;
+			cfg->nfs_server.protocol = XPRT_TRANSPORT_UDP;
 			break;
-		case Opt_sec:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			rc = nfs_parse_security_flavors(string, cfg);
-			kfree(string);
-			if (!rc) {
-				dfprintk(MOUNT, "NFS:   unrecognized "
-						"security flavor\n");
-				return -EINVAL;
-			}
+		case Opt_xprt_tcp6:
+			cfg->protofamily = AF_INET6;
+		case Opt_xprt_tcp:
+			cfg->flags |= NFS_MOUNT_TCP;
+			cfg->nfs_server.protocol = XPRT_TRANSPORT_TCP;
 			break;
-		case Opt_proto:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			token = match_token(string,
-					    nfs_xprt_protocol_tokens, args);
-
-			cfg->protofamily = AF_INET;
-			switch (token) {
-			case Opt_xprt_udp6:
-				cfg->protofamily = AF_INET6;
-			case Opt_xprt_udp:
-				cfg->flags &= ~NFS_MOUNT_TCP;
-				cfg->nfs_server.protocol = XPRT_TRANSPORT_UDP;
-				break;
-			case Opt_xprt_tcp6:
-				cfg->protofamily = AF_INET6;
-			case Opt_xprt_tcp:
-				cfg->flags |= NFS_MOUNT_TCP;
-				cfg->nfs_server.protocol = XPRT_TRANSPORT_TCP;
-				break;
-			case Opt_xprt_rdma6:
-				cfg->protofamily = AF_INET6;
-			case Opt_xprt_rdma:
-				/* vector side protocols to TCP */
-				cfg->flags |= NFS_MOUNT_TCP;
-				cfg->nfs_server.protocol = XPRT_TRANSPORT_RDMA;
-				xprt_load_transport(string);
-				break;
-			default:
-				dfprintk(MOUNT, "NFS:   unrecognized "
-						"transport protocol\n");
-				kfree(string);
-				return -EINVAL;
-			}
-			kfree(string);
+		case Opt_xprt_rdma6:
+			cfg->protofamily = AF_INET6;
+		case Opt_xprt_rdma:
+			/* vector side protocols to TCP */
+			cfg->flags |= NFS_MOUNT_TCP;
+			cfg->nfs_server.protocol = XPRT_TRANSPORT_RDMA;
+			xprt_load_transport(string);
 			break;
-		case Opt_mountproto:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			token = match_token(string,
-					    nfs_xprt_protocol_tokens, args);
+		default:
+			dfprintk(MOUNT, "NFS:   unrecognized "
+				 "transport protocol\n");
 			kfree(string);
+			return -EINVAL;
+		}
+		kfree(string);
+		break;
+	case Opt_mountproto:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		token = match_token(string,
+				    nfs_xprt_protocol_tokens, args);
+		kfree(string);
 
-			cfg->mountfamily = AF_INET;
-			switch (token) {
-			case Opt_xprt_udp6:
-				cfg->mountfamily = AF_INET6;
-			case Opt_xprt_udp:
-				cfg->mount_server.protocol = XPRT_TRANSPORT_UDP;
-				break;
-			case Opt_xprt_tcp6:
-				cfg->mountfamily = AF_INET6;
-			case Opt_xprt_tcp:
-				cfg->mount_server.protocol = XPRT_TRANSPORT_TCP;
-				break;
-			case Opt_xprt_rdma: /* not used for side protocols */
-			default:
-				dfprintk(MOUNT, "NFS:   unrecognized "
-						"transport protocol\n");
-				return -EINVAL;
-			}
+		cfg->mountfamily = AF_INET;
+		switch (token) {
+		case Opt_xprt_udp6:
+			cfg->mountfamily = AF_INET6;
+		case Opt_xprt_udp:
+			cfg->mount_server.protocol = XPRT_TRANSPORT_UDP;
 			break;
-		case Opt_addr:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			cfg->nfs_server.addrlen =
-				rpc_pton(cfg->net, string, strlen(string),
-					(struct sockaddr *)
-					&cfg->nfs_server.address,
-					sizeof(cfg->nfs_server.address));
-			kfree(string);
-			if (cfg->nfs_server.addrlen == 0)
-				goto out_invalid_address;
+		case Opt_xprt_tcp6:
+			cfg->mountfamily = AF_INET6;
+		case Opt_xprt_tcp:
+			cfg->mount_server.protocol = XPRT_TRANSPORT_TCP;
 			break;
-		case Opt_clientaddr:
-			if (nfs_get_option_str(args, &cfg->client_address))
-				goto out_nomem;
+		case Opt_xprt_rdma: /* not used for side protocols */
+		default:
+			dfprintk(MOUNT, "NFS:   unrecognized "
+				 "transport protocol\n");
+			return -EINVAL;
+		}
+		break;
+	case Opt_addr:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		cfg->nfs_server.addrlen =
+			rpc_pton(cfg->net, string, strlen(string),
+				 (struct sockaddr *)
+				 &cfg->nfs_server.address,
+				 sizeof(cfg->nfs_server.address));
+		kfree(string);
+		if (cfg->nfs_server.addrlen == 0)
+			goto out_invalid_address;
+		break;
+	case Opt_clientaddr:
+		if (nfs_get_option_str(args, &cfg->client_address))
+			goto out_nomem;
+		break;
+	case Opt_mounthost:
+		if (nfs_get_option_str(args,
+				       &cfg->mount_server.hostname))
+			goto out_nomem;
+		break;
+	case Opt_mountaddr:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		cfg->mount_server.addrlen =
+			rpc_pton(cfg->net, string, strlen(string),
+				 (struct sockaddr *)
+				 &cfg->mount_server.address,
+				 sizeof(cfg->mount_server.address));
+		kfree(string);
+		if (cfg->mount_server.addrlen == 0)
+			goto out_invalid_address;
+		break;
+	case Opt_lookupcache:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		token = match_token(string,
+				    nfs_lookupcache_tokens, args);
+		kfree(string);
+		switch (token) {
+		case Opt_lookupcache_all:
+			cfg->flags &= ~(NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE);
 			break;
-		case Opt_mounthost:
-			if (nfs_get_option_str(args,
-					       &cfg->mount_server.hostname))
-				goto out_nomem;
+		case Opt_lookupcache_positive:
+			cfg->flags &= ~NFS_MOUNT_LOOKUP_CACHE_NONE;
+			cfg->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG;
 			break;
-		case Opt_mountaddr:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			cfg->mount_server.addrlen =
-				rpc_pton(cfg->net, string, strlen(string),
-					(struct sockaddr *)
-					&cfg->mount_server.address,
-					sizeof(cfg->mount_server.address));
-			kfree(string);
-			if (cfg->mount_server.addrlen == 0)
-				goto out_invalid_address;
+		case Opt_lookupcache_none:
+			cfg->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE;
 			break;
-		case Opt_lookupcache:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			token = match_token(string,
-					nfs_lookupcache_tokens, args);
-			kfree(string);
-			switch (token) {
-				case Opt_lookupcache_all:
-					cfg->flags &= ~(NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE);
-					break;
-				case Opt_lookupcache_positive:
-					cfg->flags &= ~NFS_MOUNT_LOOKUP_CACHE_NONE;
-					cfg->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG;
-					break;
-				case Opt_lookupcache_none:
-					cfg->flags |= NFS_MOUNT_LOOKUP_CACHE_NONEG|NFS_MOUNT_LOOKUP_CACHE_NONE;
-					break;
-				default:
-					dfprintk(MOUNT, "NFS:   invalid "
-							"lookupcache argument\n");
-					return -EINVAL;
-			};
+		default:
+			dfprintk(MOUNT, "NFS:   invalid "
+				 "lookupcache argument\n");
+			return -EINVAL;
+		};
+		break;
+	case Opt_fscache_uniq:
+		if (nfs_get_option_str(args, &cfg->fscache_uniq))
+			goto out_nomem;
+		cfg->options |= NFS_OPTION_FSCACHE;
+		break;
+	case Opt_local_lock:
+		string = match_strdup(args);
+		if (string == NULL)
+			goto out_nomem;
+		token = match_token(string, nfs_local_lock_tokens,
+				    args);
+		kfree(string);
+		switch (token) {
+		case Opt_local_lock_all:
+			cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK |
+				       NFS_MOUNT_LOCAL_FCNTL);
 			break;
-		case Opt_fscache_uniq:
-			if (nfs_get_option_str(args, &cfg->fscache_uniq))
-				goto out_nomem;
-			cfg->options |= NFS_OPTION_FSCACHE;
+		case Opt_local_lock_flock:
+			cfg->flags |= NFS_MOUNT_LOCAL_FLOCK;
 			break;
-		case Opt_local_lock:
-			string = match_strdup(args);
-			if (string == NULL)
-				goto out_nomem;
-			token = match_token(string, nfs_local_lock_tokens,
-					args);
-			kfree(string);
-			switch (token) {
-			case Opt_local_lock_all:
-				cfg->flags |= (NFS_MOUNT_LOCAL_FLOCK |
-					       NFS_MOUNT_LOCAL_FCNTL);
-				break;
-			case Opt_local_lock_flock:
-				cfg->flags |= NFS_MOUNT_LOCAL_FLOCK;
-				break;
-			case Opt_local_lock_posix:
-				cfg->flags |= NFS_MOUNT_LOCAL_FCNTL;
-				break;
-			case Opt_local_lock_none:
-				cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
-						NFS_MOUNT_LOCAL_FCNTL);
-				break;
-			default:
-				dfprintk(MOUNT, "NFS:	invalid	"
-						"local_lock argument\n");
-				return -EINVAL;
-			};
+		case Opt_local_lock_posix:
+			cfg->flags |= NFS_MOUNT_LOCAL_FCNTL;
 			break;
+		case Opt_local_lock_none:
+			cfg->flags &= ~(NFS_MOUNT_LOCAL_FLOCK |
+					NFS_MOUNT_LOCAL_FCNTL);
+			break;
+		default:
+			dfprintk(MOUNT, "NFS:	invalid	"
+				 "local_lock argument\n");
+			return -EINVAL;
+		};
+		break;
 
 		/*
 		 * Special options
 		 */
-		case Opt_sloppy:
-			cfg->sloppy = 1;
-			dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
-			break;
-		case Opt_userspace:
-		case Opt_deprecated:
-			dfprintk(MOUNT, "NFS:   ignoring mount option "
-					"'%s'\n", p);
-			break;
+	case Opt_sloppy:
+		cfg->sloppy = 1;
+		dfprintk(MOUNT, "NFS:   relaxing parsing rules\n");
+		break;
+	case Opt_userspace:
+	case Opt_deprecated:
+		dfprintk(MOUNT, "NFS:   ignoring mount option "
+			 "'%s'\n", p);
+		break;
 
-		default:
-			dfprintk(MOUNT, "NFS:   unrecognized mount option "
-					"'%s'\n", p);
-			return -EINVAL;
-		}
+	default:
+		dfprintk(MOUNT, "NFS:   unrecognized mount option "
+			 "'%s'\n", p);
+		return -EINVAL;
 	}
 
 	return 0;

[toc] | [prev] | [next] | [standalone]


#1642670 — Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-16 18:40 +0200
SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
Message-ID<tHNFT-7VJ-13@gated-at.bofh.it>
In reply to#1641798
Miklos Szeredi <mszeredi@redhat.com> wrote:

> One way to split this large patch up into more managable chunks would be:
> 
>  1) common infrastructure
>  2) new mount related changes
>  3) reconfig (remount) related changes
> 
> Would that work?

The problem is that remount seems to generally use the same parsing code as
the new-mount entry point.

Before considering how to split it, can we consider whether to roll patches 20
and 21 into the preceding patches?

>   (a) new mount with new super block created
>   (b) new mount with existing super block reused
>   (c) remount

(b) is internal-only at the moment, used by NFS submounts as triggered by
automounts.  There isn't currently any way to supply mount options to this.

>   2) modify options ("foo" turns option on, "nofoo" turns it off)

Not all options are binary and some options may be mandatory.

> The surprising thing here is that we do (a) and (b) via the same route
> and (a) and (c) via a different ones.  This doesn't feel right.

You need to look at it like this:

	Case	Options	Ref	Call	Modify
		        super	sget	super
	=======	=======	=======	=======	=======
	a	Y	-	Y	-
	b	-	Y	Y	-
	c	Y	[1]	-	Y

[1] We don't have a separate reference sb, only the one we're going to modify,
    but we can preload the sb_config from that.

(a) and (b) have the same action.

>   i) options that determine the sb instance (such as the blockdev or
> the server IP address)
>   ii) subpath: this can determine the sb as well as the subtree to use
>   iii) options that can be changed while sb in use
>   iv) ???

Ah - but some of these options have to be set *inside* sget() or before the
superblock becomes live, even the ones that can be changed in-flight.

> Would it make sense to make the "new mount" case be
> 
>   A) find or create sb based on (i) and (ii) options
>   B) reconfigure the resulting sb based on (iii) options

You would *have* to do the reconfiguration before making the superblock live
to prevent config/use races, and some options in (iii) may be required during
sget(), or even before you get as far as calling sget() (say you need to
access a server).

> This would make legacy new mount be: (A) + if new then (B).  And
> legacy remount just (B).

It's not obvious that this is sufficiently equivalent from your brief
description.

> Also I think silently ignoring options is not always the right answer.

Example?

Do you mean like the NFS 'sloppy' option?  I've noted that that might be best
handled in userspace.

> > +       int (*remount_fs_sc) (struct super_block *, struct sb_config *);
> 
> How about reconfig_fs() or just reconfig()?

Sure.

> > + (*) struct dentry *(*mount)(struct sb_config *sc);
> 
> I'd be much happier with "get_root()" or something.

Changed in patch 21 to ->get_tree() as suggested by Al.  Having looked over
the code, I'm tempted to change it back to ->mount() as being more obvious.

> > +               err = parse_monolithic_mount_data(sc, data);
> > +               if (err < 0)
> > +                       goto err_sc;
> 
> If filesystem defines ->monolithic_mount_data() who is responsible for
> calling the security hook?

Which security hook?  security_sb_remount()?

Note this code has changed in patch 20.  I should update security_sb_remount()
to take an sb_config and call it in all paths.

> Largely duplicated do_new_mount_sc().  What's the point?

Legacy vs new.  Fixed in patch 20.

> Lots of these are not superblock options, and should be moved over to
> the forbidden ones.  Look at do_mount() for a hint.

I still have to support legacy mount option parsing.  Do I actually see these
in legacy mount(2)?  Or are they weeded out by mount(8)?

David

[toc] | [prev] | [next] | [standalone]


#1643081 — Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]

FromMiklos Szeredi <mszeredi@redhat.com>
Date2017-05-17 10:00 +0200
SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
Message-ID<tI22e-fo-9@gated-at.bofh.it>
In reply to#1642670
On Tue, May 16, 2017 at 6:33 PM, David Howells <dhowells@redhat.com> wrote:
> Miklos Szeredi <mszeredi@redhat.com> wrote:
>
>> One way to split this large patch up into more managable chunks would be:
>>
>>  1) common infrastructure
>>  2) new mount related changes
>>  3) reconfig (remount) related changes
>>
>> Would that work?
>
> The problem is that remount seems to generally use the same parsing code as
> the new-mount entry point.

You are talking about fs code?

I was just referring to this single patch, not the others.

>
> Before considering how to split it, can we consider whether to roll patches 20
> and 21 into the preceding patches?
>
>>   (a) new mount with new super block created
>>   (b) new mount with existing super block reused
>>   (c) remount
>
> (b) is internal-only at the moment, used by NFS submounts as triggered by
> automounts.  There isn't currently any way to supply mount options to this.

And all blockdev based fs.

>>   2) modify options ("foo" turns option on, "nofoo" turns it off)
>
> Not all options are binary and some options may be mandatory.

Right, I was simplifying.

>
>> The surprising thing here is that we do (a) and (b) via the same route
>> and (a) and (c) via a different ones.  This doesn't feel right.
>
> You need to look at it like this:
>
>         Case    Options Ref     Call    Modify
>                         super   sget    super
>         ======= ======= ======= ======= =======
>         a       Y       -       Y       -
>         b       -       Y       Y       -
>         c       Y       [1]     -       Y
>
> [1] We don't have a separate reference sb, only the one we're going to modify,
>     but we can preload the sb_config from that.
>
> (a) and (b) have the same action.
>
>>   i) options that determine the sb instance (such as the blockdev or
>> the server IP address)
>>   ii) subpath: this can determine the sb as well as the subtree to use
>>   iii) options that can be changed while sb in use
>>   iv) ???
>
> Ah - but some of these options have to be set *inside* sget() or before the
> superblock becomes live, even the ones that can be changed in-flight.

That would be the "???" category.  Any concrete examples?

>
>> Would it make sense to make the "new mount" case be
>>
>>   A) find or create sb based on (i) and (ii) options
>>   B) reconfigure the resulting sb based on (iii) options
>
> You would *have* to do the reconfiguration before making the superblock live
> to prevent config/use races,

Indeed, the actual attaching into the mount namespace would be the
final step.  I deliberately left it out, because it's largely
orthogonal to the superblock config question.

> and some options in (iii) may be required during
> sget(), or even before you get as far as calling sget() (say you need to
> access a server).
>
>> This would make legacy new mount be: (A) + if new then (B).  And
>> legacy remount just (B).
>
> It's not obvious that this is sufficiently equivalent from your brief
> description.

It's not obvious.  I'm just trying to explore the design space to fix
as much idiocy in the current one as possible.

>
>> Also I think silently ignoring options is not always the right answer.
>
> Example?

mount /dev/sda -oacl /mnt
mount /dev/sda -onoacl /mnt2

>
> Do you mean like the NFS 'sloppy' option?  I've noted that that might be best
> handled in userspace.
>
>> > +       int (*remount_fs_sc) (struct super_block *, struct sb_config *);
>>
>> How about reconfig_fs() or just reconfig()?
>
> Sure.
>
>> > + (*) struct dentry *(*mount)(struct sb_config *sc);
>>
>> I'd be much happier with "get_root()" or something.
>
> Changed in patch 21 to ->get_tree() as suggested by Al.  Having looked over
> the code, I'm tempted to change it back to ->mount() as being more obvious.

Some definitions of mount:

 - to increase in amount or extent
 - to seat oneself (as on a horse) for riding
 - to climb on top of for copulation
 - to launch and carry out (something, such as an assault or a campaign)
 - to attach to a support

No really good match for what this method is doing.  We could call it
->get_tree_to_mount(), but calling it just ->mount() implies that it's
doing the mounting, which it is not.

>
>> > +               err = parse_monolithic_mount_data(sc, data);
>> > +               if (err < 0)
>> > +                       goto err_sc;
>>
>> If filesystem defines ->monolithic_mount_data() who is responsible for
>> calling the security hook?
>
> Which security hook?  security_sb_remount()?
>
> Note this code has changed in patch 20.  I should update security_sb_remount()
> to take an sb_config and call it in all paths.
>
>> Largely duplicated do_new_mount_sc().  What's the point?
>
> Legacy vs new.  Fixed in patch 20.
>
>> Lots of these are not superblock options, and should be moved over to
>> the forbidden ones.  Look at do_mount() for a hint.
>
> I still have to support legacy mount option parsing.  Do I actually see these
> in legacy mount(2)?  Or are they weeded out by mount(8)?

You are thinking on the wrong level.  Of course mount(2) needs to
handle MS_NOSUID et al.  But it's doing it now, and it isn't parsing
"nosuid", just translating MS_NOSUID to MNT_NOSUID.   For the fsopen()
case you won't need to parse "nosuid" because that's a flag for
fsmount().

The only thing fsmount() should take from the sc is the root_dentry.
It should be equivalent to what currently is a bind mount, except it
should be able to fully configure the new mount.

I'm still hoping we can move subpath handling completely to fsmount()
in which case it would just take a struct super_block.  But that would
have to start with lots of filesystem work (not just NFS but CEPH,
CIFS, etc..).

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]


#1643280 — Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-17 13:40 +0200
SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
Message-ID<tI5t8-2sV-17@gated-at.bofh.it>
In reply to#1643081
Miklos Szeredi <mszeredi@redhat.com> wrote:

> > (b) is internal-only at the moment, used by NFS submounts as triggered by
> > automounts.  There isn't currently any way to supply mount options to this.
> 
> And all blockdev based fs.

I see what you're getting at.  In which case there are more cases:

  (a) new mount, new sb struct with no source (eg. procfs, sysfs, tmpfs)
  (b) new mount, new sb struct, params loaded from filesystem data (eg. bdev)
  (c) new mount, new sb struct, params derived from parent (eg. NFS automount)
  (d) new mount, shared extant sb struct
  (e) remount

In the case of (d) where we're attempting to make another mount for an extant
super_block struct and we need to check the consistency of the parameters.

> > Ah - but some of these options have to be set *inside* sget() or before the
> > superblock becomes live, even the ones that can be changed in-flight.
> 
> That would be the "???" category.  Any concrete examples?

NFS is a good example.  You need parameters that indicate the server to talk
to and specify I/O parameters before you even get the superblock as you have
to talk to the server first.  I think this is particularly true of NFSv2/3
where you need to talk to mountd.

This would also be true of AFS.  There you have to access the network to look
up the volume ID before you can call sget() as the volume ID is part of the
index key to the set of super_block structs.

Further, some of these values (I/O parameters in NFS's case, for example) form
part of the super_block struct index key, so you have to set those inside
sget()'s set callback.

> >> Also I think silently ignoring options is not always the right answer.
> >
> > Example?
> 
> mount /dev/sda -oacl /mnt
> mount /dev/sda -onoacl /mnt2

So you'd like to give an error or a warning if ACLs are not supported, either
by the filesystem or the kernel as a whole?

> No really good match for what this method is doing.  We could call it
> ->get_tree_to_mount(), but calling it just ->mount() implies that it's
> doing the mounting, which it is not.

Yes, but my point is that it's part of the mount procedure.  We are, I assume,
intending to try and mount the thing at some point.  I can leave it as
->get_tree() for the moment.

> You are thinking on the wrong level.  Of course mount(2) needs to
> handle MS_NOSUID et al.  But it's doing it now, and it isn't parsing
> "nosuid", just translating MS_NOSUID to MNT_NOSUID.

Ummm...  That's done by the parser in this case, so effectively it is.

> For the fsopen() case you won't need to parse "nosuid" because that's a flag
> for fsmount().

Whilst this is true, that means that the parser has to operate differently in
the mount(2) and fsopen(2) cases - which I was trying to avoid.  I guess I can
set a flag in the sb_config struct to indicate the source and then split out
these options into an only-for-mount(2) list.

> The only thing fsmount() should take from the sc is the root_dentry.
> It should be equivalent to what currently is a bind mount, except it
> should be able to fully configure the new mount.

It needs to take the device name as well.  I wonder if it would be possible to
store the device name on the superblock and then leave a path-in-mount in the
vfsmount struct to fabricate a <source>:/<path> later.  Though this would
change the behaviour if someone did:

	mknod /dev/foo b 8 1
	mknod /dev/bar b 8 1
	mount /dev/foo /mnt/foo
	mount /dev/bar /mnt/bar

as /proc/mounts would now show /dev/foo for /mnt/bar.

Also, I guess the subtype should be wangled in the superblock-getting code
(vfs_get_tree() as of patch 21) rather than in do_new_mount_sc().  If I do
that, then it may be that do_new_mount_sc() only needs the root dentry pointer
and not the sb_config pointer (except for error string passing).

> I'm still hoping we can move subpath handling completely to fsmount()
> in which case it would just take a struct super_block.  But that would
> have to start with lots of filesystem work (not just NFS but CEPH,
> CIFS, etc..).

That would be nice, though NFSv2/3 might be tricky.

David

[toc] | [prev] | [next] | [standalone]


#1643946 — Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]

FromMiklos Szeredi <mszeredi@redhat.com>
Date2017-05-18 10:20 +0200
SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
Message-ID<tIoP7-7E3-3@gated-at.bofh.it>
In reply to#1643280
On Wed, May 17, 2017 at 1:31 PM, David Howells <dhowells@redhat.com> wrote:
> Miklos Szeredi <mszeredi@redhat.com> wrote:
>
>> > (b) is internal-only at the moment, used by NFS submounts as triggered by
>> > automounts.  There isn't currently any way to supply mount options to this.
>>
>> And all blockdev based fs.
>
> I see what you're getting at.  In which case there are more cases:
>
>   (a) new mount, new sb struct with no source (eg. procfs, sysfs, tmpfs)
>   (b) new mount, new sb struct, params loaded from filesystem data (eg. bdev)
>   (c) new mount, new sb struct, params derived from parent (eg. NFS automount)
>   (d) new mount, shared extant sb struct
>   (e) remount
>
> In the case of (d) where we're attempting to make another mount for an extant
> super_block struct and we need to check the consistency of the parameters.

Yes.  Current behavior seems to just ignore given options (except
MS_RDONLY) in that case, so we need to keep that possibility.

Also I think it would be good to allow selecting when superblock is created:

  - non-exclusive create: if exists return it, if not create it
  - exclusive create: only create if non-existent
  - non-create: only return if exists

>
>> > Ah - but some of these options have to be set *inside* sget() or before the
>> > superblock becomes live, even the ones that can be changed in-flight.
>>
>> That would be the "???" category.  Any concrete examples?
>
> NFS is a good example.  You need parameters that indicate the server to talk
> to and specify I/O parameters before you even get the superblock as you have
> to talk to the server first.  I think this is particularly true of NFSv2/3
> where you need to talk to mountd.
>
> This would also be true of AFS.  There you have to access the network to look
> up the volume ID before you can call sget() as the volume ID is part of the
> index key to the set of super_block structs.
>
> Further, some of these values (I/O parameters in NFS's case, for example) form
> part of the super_block struct index key, so you have to set those inside
> sget()'s set callback.

So what I propose is:

 1) call ->parse_option()

      would get indication what we are trying to do (find and/or
create and/or reconfig)

      this step is optional, the the filesystem type could possibly be
enough for the following steps

 2) call ->get_tree()

      pass sc containing parsed options and flags controlling the
creation of the superblock (create/exclusive)

      this step is optional, not called if we are given an sb to work
with (i.e. only reconfig)

 3) call ->reconfig()

      pass sc containing parsed options

      this step is optional, we might be instructed just to find or
create the sb

>
>> >> Also I think silently ignoring options is not always the right answer.
>> >
>> > Example?
>>
>> mount /dev/sda -oacl /mnt
>> mount /dev/sda -onoacl /mnt2
>
> So you'd like to give an error or a warning if ACLs are not supported, either
> by the filesystem or the kernel as a whole?

What I was getting at is that the second mount will ignore the "noacl"
option.  It's not something we apparently care much about (but will
definitely want to keep as back-compat thing for the mount(2)
interface).  But for the new interface I think we need something less
crazy.  One solution would be the exclusive create, which doesn't have
this problem. Maybe that's enough; not sure if we need anything more
sophisticated.

>> You are thinking on the wrong level.  Of course mount(2) needs to
>> handle MS_NOSUID et al.  But it's doing it now, and it isn't parsing
>> "nosuid", just translating MS_NOSUID to MNT_NOSUID.
>
> Ummm...  That's done by the parser in this case, so effectively it is.

Where exactly?  You are not touching do_mount(), which is where the
MS_*** -> MNT_*** translation is done.


>> For the fsopen() case you won't need to parse "nosuid" because that's a flag
>> for fsmount().
>
> Whilst this is true, that means that the parser has to operate differently in
> the mount(2) and fsopen(2) cases - which I was trying to avoid.

I don't get it.  We never passed MNT_* options as strings to the
kernel.  That was parsed by mount(8) and translated to MS_* flags.
So how would mount(2) and fsopen(2) need to operate differently
regarding parsing MNT_* options, when we want neither to do it?

>> The only thing fsmount() should take from the sc is the root_dentry.
>> It should be equivalent to what currently is a bind mount, except it
>> should be able to fully configure the new mount.
>
> It needs to take the device name as well.  I wonder if it would be possible to
> store the device name on the superblock and then leave a path-in-mount in the

Ah, mnt_devname.  The device name as just a special type of option and
as such should be stored in the superblock.

> vfsmount struct to fabricate a <source>:/<path> later.  Though this would
> change the behaviour if someone did:
>
>         mknod /dev/foo b 8 1
>         mknod /dev/bar b 8 1
>         mount /dev/foo /mnt/foo
>         mount /dev/bar /mnt/bar
>
> as /proc/mounts would now show /dev/foo for /mnt/bar.

I'd very much hope this doesn't introduce regressions, but if it did,
then we'd have to go back to using mnt_devname...

> Also, I guess the subtype should be wangled in the superblock-getting code
> (vfs_get_tree() as of patch 21) rather than in do_new_mount_sc().  If I do
> that, then it may be that do_new_mount_sc() only needs the root dentry pointer
> and not the sb_config pointer (except for error string passing).

Would be nice.

And hopefully error string passing can be made generic and be moved
out of this set.

Thanks,
Miklos

[toc] | [prev] | [next] | [standalone]


#1645628 — Re: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]

FromDavid Howells <dhowells@redhat.com>
Date2017-05-19 16:10 +0200
SubjectRe: [PATCH 06/21] VFS: Introduce a superblock configuration context [ver #3]
Message-ID<tIQLo-3eE-17@gated-at.bofh.it>
In reply to#1643946
Miklos Szeredi <mszeredi@redhat.com> wrote:

> Yes.  Current behavior seems to just ignore given options (except
> MS_RDONLY) in that case, so we need to keep that possibility.

Yeah.  I wonder if we really should be consistency checking some parameters in
some filesystems - or, at least, offering the opportunity.

> Also I think it would be good to allow selecting when superblock is created:
> 
>   - non-exclusive create: if exists return it, if not create it
>   - exclusive create: only create if non-existent
>   - non-create: only return if exists

I quite like that idea.  Use O_CREAT and O_EXCL?  Probably better to define a
new flag space for fsopen() rather than trying to share with open().  I'm not
sure how likely it would be to be used, though.

> So what I propose is:
> 
>  1) call ->parse_option()
> 
>       would get indication what we are trying to do (find and/or
> create and/or reconfig)
> 
>       this step is optional, the the filesystem type could possibly be
> enough for the following steps
> 
>  2) call ->get_tree()
> 
>       pass sc containing parsed options and flags controlling the
> creation of the superblock (create/exclusive)
> 
>       this step is optional, not called if we are given an sb to work
> with (i.e. only reconfig)

No.  We have to call this to get the root dentry.  Whether or not it creates a
superblock - or even if it creates a superblock in someone else's filesystem
(the cpuset fs, for example) - is immaterial.

Further, we aren't given information as to whether the superblock was created
for us or not - though that can be changed.

Even further, I think by the time this returns, the superblock should be
live.  It will be live if we're reusing it, though we can get s_umount to
prevent a race.

>  3) call ->reconfig()
> 
>       pass sc containing parsed options
> 
>       this step is optional, we might be instructed just to find or
> create the sb

Actually, it's arguable that we *shouldn't* be calling this if the superblock
already exists - otherwise we may end up changing the parameters someone else
has set.

For mount(2), for most filesystems, we have to leave the active parameters
unaltered for compatibility.  For fsopen() I'm willing to add a consistency
check - but there probably has to be a flag to waive that as otherwise you
can't mount without determining what the other party's parameters were.

> I don't get it.  We never passed MNT_* options as strings to the
> kernel.

You're right.  I've moved all those flags over to the forbidden list.

> Ah, mnt_devname.  The device name as just a special type of option and
> as such should be stored in the superblock.

I'll leave that for now and deal with it later.  We have to be careful not to
break userspace by changing what's seen in /proc/mounts.

David

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web