Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1635100 > unrolled thread
| Started by | David Howells <dhowells@redhat.com> |
|---|---|
| First post | 2017-05-03 18:10 +0200 |
| Last post | 2017-05-08 19:10 +0200 |
| Articles | 20 on this page of 61 — 10 participants |
Back to article view | Back to linux.kernel
[RFC][PATCH 0/9] VFS: Introduce mount context David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
[PATCH 1/9] Provide a function to create a NUL-terminated string from unterminated data David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
Re: [PATCH 1/9] Provide a function to create a NUL-terminated string from unterminated data Jeff Layton <jlayton@poochiereds.net> - 2017-05-03 19:00 +0200
Re: [PATCH 1/9] Provide a function to create a NUL-terminated string from unterminated data Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2017-05-03 21:30 +0200
Re: [PATCH 1/9] Provide a function to create a NUL-terminated string from unterminated data David Howells <dhowells@redhat.com> - 2017-05-03 22:20 +0200
[PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Jeff Layton <jlayton@poochiereds.net> - 2017-05-03 20:40 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-03 20:50 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2017-05-03 22:50 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-04 15:00 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-04 15:00 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Karel Zak <kzak@redhat.com> - 2017-05-04 12:50 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-04 15:10 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Karel Zak <kzak@redhat.com> - 2017-05-04 15:40 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Jeff Layton <jlayton@redhat.com> - 2017-05-09 20:50 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount Miklos Szeredi <mszeredi@redhat.com> - 2017-05-08 17:20 +0200
Re: [PATCH 4/9] Implement fsopen() to prepare for a mount David Howells <dhowells@redhat.com> - 2017-05-09 01:20 +0200
[PATCH 5/9] Implement fsmount() to effect a pre-configured mount David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
[PATCH 2/9] Clean up whitespace in fs/namespace.c David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
[PATCH 8/9] proc: Support the mount context in procfs David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
[PATCH 7/9] procfs: Move proc_fill_super() to fs/proc/root.c David Howells <dhowells@redhat.com> - 2017-05-03 18:10 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Jeff Layton <jlayton@poochiereds.net> - 2017-05-03 18:50 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context David Howells <dhowells@redhat.com> - 2017-05-03 19:00 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Jeff Layton <jlayton@poochiereds.net> - 2017-05-03 19:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Joe Perches <joe@perches.com> - 2017-05-03 20:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-03 20:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Joe Perches <joe@perches.com> - 2017-05-03 20:50 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-03 22:20 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Matthew Wilcox <willy@infradead.org> - 2017-05-03 22:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-03 23:20 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Joe Perches <joe@perches.com> - 2017-05-03 23:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Julia Lawall <julia.lawall@lip6.fr> - 2017-05-04 08:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-04 11:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Joe Perches <joe@perches.com> - 2017-05-04 16:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2017-05-03 23:50 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-04 12:30 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-05 16:40 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context David Howells <dhowells@redhat.com> - 2017-05-05 17:50 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-08 10:30 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context David Howells <dhowells@redhat.com> - 2017-05-08 10:40 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-08 10:50 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-08 17:10 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-09 01:00 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-09 10:10 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-09 11:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-09 13:10 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-09 11:50 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-09 14:10 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Jeff Layton <jlayton@redhat.com> - 2017-05-09 21:00 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-10 09:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-10 10:10 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Jeff Layton <jlayton@redhat.com> - 2017-05-10 15:30 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-10 15:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Jeff Layton <jlayton@redhat.com> - 2017-05-10 15:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-10 15:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-10 15:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Jeff Layton <jlayton@redhat.com> - 2017-05-10 15:50 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context David Howells <dhowells@redhat.com> - 2017-05-09 12:00 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Miklos Szeredi <mszeredi@redhat.com> - 2017-05-09 14:40 +0200
Re: [PATCH 3/9] VFS: Introduce a mount context Karel Zak <kzak@redhat.com> - 2017-05-10 14:50 +0200
Re: [RFC][PATCH 0/9] VFS: Introduce mount context Djalal Harouni <tixxdz@gmail.com> - 2017-05-08 19:10 +0200
Page 2 of 4 — ← Prev page 1 [2] 3 4 Next page →
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-03 18:10 +0200 |
| Subject | [PATCH 7/9] procfs: Move proc_fill_super() to fs/proc/root.c |
| Message-ID | <tD50L-7GP-49@gated-at.bofh.it> |
| In reply to | #1635100 |
Move proc_fill_super() to fs/proc/root.c as that's where the other
superblock stuff is.
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/proc/inode.c | 48 +-----------------------------------------------
fs/proc/internal.h | 4 +---
fs/proc/root.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 49 insertions(+), 51 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 2cc7a8030275..194fa2d13b7e 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -22,7 +22,6 @@
#include <linux/seq_file.h>
#include <linux/slab.h>
#include <linux/mount.h>
-#include <linux/magic.h>
#include <linux/uaccess.h>
@@ -113,7 +112,7 @@ static int proc_show_options(struct seq_file *seq, struct dentry *root)
return 0;
}
-static const struct super_operations proc_sops = {
+const struct super_operations proc_sops = {
.alloc_inode = proc_alloc_inode,
.destroy_inode = proc_destroy_inode,
.drop_inode = generic_delete_inode,
@@ -470,48 +469,3 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
pde_put(de);
return inode;
}
-
-int proc_fill_super(struct super_block *s, void *data, int silent)
-{
- struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
- struct inode *root_inode;
- int ret;
-
- if (!proc_parse_options(data, ns))
- return -EINVAL;
-
- /* User space would break if executables or devices appear on proc */
- s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
- s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
- s->s_blocksize = 1024;
- s->s_blocksize_bits = 10;
- s->s_magic = PROC_SUPER_MAGIC;
- s->s_op = &proc_sops;
- s->s_time_gran = 1;
-
- /*
- * procfs isn't actually a stacking filesystem; however, there is
- * too much magic going on inside it to permit stacking things on
- * 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) {
- pr_err("proc_fill_super: get root inode failed\n");
- return -ENOMEM;
- }
-
- s->s_root = d_make_root(root_inode);
- if (!s->s_root) {
- pr_err("proc_fill_super: allocate dentry failed\n");
- return -ENOMEM;
- }
-
- ret = proc_setup_self(s);
- if (ret) {
- return ret;
- }
- return proc_setup_thread_self(s);
-}
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index c5ae09b6c726..b681533f59dd 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -197,13 +197,12 @@ struct pde_opener {
struct completion *c;
};
extern const struct inode_operations proc_link_inode_operations;
-
extern const struct inode_operations proc_pid_link_inode_operations;
+extern const struct super_operations proc_sops;
extern void proc_init_inodecache(void);
void set_proc_pid_nlink(void);
extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
-extern int proc_fill_super(struct super_block *, void *data, int flags);
extern void proc_entry_rundown(struct proc_dir_entry *);
/*
@@ -261,7 +260,6 @@ static inline void proc_tty_init(void) {}
* root.c
*/
extern struct proc_dir_entry proc_root;
-extern int proc_parse_options(char *options, struct pid_namespace *pid);
extern void proc_self_init(void);
extern int proc_remount(struct super_block *, int *, char *);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index deecb397daa3..ff2e810e9e64 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -22,6 +22,7 @@
#include <linux/pid_namespace.h>
#include <linux/parser.h>
#include <linux/cred.h>
+#include <linux/magic.h>
#include "internal.h"
@@ -35,7 +36,7 @@ static const match_table_t tokens = {
{Opt_err, NULL},
};
-int proc_parse_options(char *options, struct pid_namespace *pid)
+static int proc_parse_options(char *options, struct pid_namespace *pid)
{
char *p;
substring_t args[MAX_OPT_ARGS];
@@ -77,6 +78,51 @@ int proc_parse_options(char *options, struct pid_namespace *pid)
return 1;
}
+static int proc_fill_super(struct super_block *s, void *data, int silent)
+{
+ struct pid_namespace *ns = get_pid_ns(s->s_fs_info);
+ struct inode *root_inode;
+ int ret;
+
+ if (!proc_parse_options(data, ns))
+ return -EINVAL;
+
+ /* User space would break if executables or devices appear on proc */
+ s->s_iflags |= SB_I_USERNS_VISIBLE | SB_I_NOEXEC | SB_I_NODEV;
+ s->s_flags |= MS_NODIRATIME | MS_NOSUID | MS_NOEXEC;
+ s->s_blocksize = 1024;
+ s->s_blocksize_bits = 10;
+ s->s_magic = PROC_SUPER_MAGIC;
+ s->s_op = &proc_sops;
+ s->s_time_gran = 1;
+
+ /*
+ * procfs isn't actually a stacking filesystem; however, there is
+ * too much magic going on inside it to permit stacking things on
+ * 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) {
+ pr_err("proc_fill_super: get root inode failed\n");
+ return -ENOMEM;
+ }
+
+ s->s_root = d_make_root(root_inode);
+ if (!s->s_root) {
+ pr_err("proc_fill_super: allocate dentry failed\n");
+ return -ENOMEM;
+ }
+
+ ret = proc_setup_self(s);
+ if (ret) {
+ return ret;
+ }
+ return proc_setup_thread_self(s);
+}
+
int proc_remount(struct super_block *sb, int *flags, char *data)
{
struct pid_namespace *pid = sb->s_fs_info;
[toc] | [prev] | [next] | [standalone]
| From | Jeff Layton <jlayton@poochiereds.net> |
|---|---|
| Date | 2017-05-03 18:50 +0200 |
| Message-ID | <tD5Dr-7Ux-9@gated-at.bofh.it> |
| In reply to | #1635100 |
On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote:
> Here are a set of patches to create a mount context prior to setting up a
> new mount, populating it with the parsed options/binary data 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.
>
Nice work!
> I've implemented mount context handling for procfs and nfs.
>
> Further developments:
>
> (*) 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.
>
':' is a legitimate character in a path component. How will you
distinguish that case?
> (*) Move the pid_ns pointer from struct mount_context to struct
> proc_mount_context as I'm not sure it's necessary for anything other
> than procfs.
>
> (*) 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?
>
Now that's an interesting idea.
> (*) 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 (9):
> Provide a function to create a NUL-terminated string from unterminated data
> Clean up whitespace in fs/namespace.c
> VFS: Introduce a mount 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: Support the mount context in procfs
> NFS: Support the mount context and fsopen()
>
>
> Documentation/filesystems/mounting.txt | 445 ++++++++
> arch/x86/entry/syscalls/syscall_32.tbl | 2
> arch/x86/entry/syscalls/syscall_64.tbl | 2
> fs/Makefile | 3
> fs/fsopen.c | 295 +++++
> fs/internal.h | 2
> fs/mount.h | 3
> fs/mount_context.c | 343 ++++++
> fs/namespace.c | 367 ++++++-
> fs/nfs/Makefile | 2
> fs/nfs/client.c | 18
> fs/nfs/internal.h | 127 +-
> fs/nfs/mount.c | 1539 ++++++++++++++++++++++++++++
> fs/nfs/namespace.c | 75 +
> fs/nfs/nfs3_fs.h | 2
> fs/nfs/nfs3client.c | 6
> fs/nfs/nfs3proc.c | 1
> fs/nfs/nfs4_fs.h | 4
> fs/nfs/nfs4client.c | 80 +
> fs/nfs/nfs4namespace.c | 207 ++--
> fs/nfs/nfs4proc.c | 1
> fs/nfs/nfs4super.c | 184 ++-
> fs/nfs/proc.c | 1
> fs/nfs/super.c | 1729 ++------------------------------
> fs/proc/inode.c | 50 -
> fs/proc/internal.h | 6
> fs/proc/root.c | 194 +++-
> fs/super.c | 50 +
> include/linux/fs.h | 11
> include/linux/lsm_hooks.h | 43 +
> include/linux/mount.h | 67 +
> include/linux/nfs_xdr.h | 7
> include/linux/security.h | 35 +
> include/linux/string.h | 1
> include/linux/syscalls.h | 2
> include/uapi/linux/magic.h | 1
> kernel/sys_ni.c | 4
> mm/util.c | 22
> samples/fsmount/test-fsmount.c | 79 +
> security/security.c | 39 +
> security/selinux/hooks.c | 192 ++++
> 41 files changed, 4148 insertions(+), 2093 deletions(-)
> create mode 100644 Documentation/filesystems/mounting.txt
> create mode 100644 fs/fsopen.c
> create mode 100644 fs/mount_context.c
> create mode 100644 fs/nfs/mount.c
> create mode 100644 samples/fsmount/test-fsmount.c
>
--
Jeff Layton <jlayton@poochiereds.net>
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-03 19:00 +0200 |
| Message-ID | <tD5N7-7Y4-5@gated-at.bofh.it> |
| In reply to | #1635135 |
Jeff Layton <jlayton@poochiereds.net> wrote: > > (*) 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. > > > > ':' is a legitimate character in a path component. How will you > distinguish that case? Fair point. Could instead do something like: mount /dev/sda1 /mnt -o subroot=/foo/bar or just limit it to the fsopen interface. David
[toc] | [prev] | [next] | [standalone]
| From | Jeff Layton <jlayton@poochiereds.net> |
|---|---|
| Date | 2017-05-03 19:30 +0200 |
| Message-ID | <tD6g9-8nI-7@gated-at.bofh.it> |
| In reply to | #1635137 |
On Wed, 2017-05-03 at 17:50 +0100, David Howells wrote: > Jeff Layton <jlayton@poochiereds.net> wrote: > > > > (*) 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. > > > > > > > ':' is a legitimate character in a path component. How will you > > distinguish that case? > > Fair point. Could instead do something like: > > mount /dev/sda1 /mnt -o subroot=/foo/bar > > or just limit it to the fsopen interface. > > Yeah, something like that would certainly work. I like the basic idea though of combining a mount and bind mount for local fs'. -- Jeff Layton <jlayton@poochiereds.net>
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-05-03 20:30 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD7ce-Ai-9@gated-at.bofh.it> |
| In reply to | #1635100 |
On Wed, 2017-05-03 at 14:13 -0400, Jeff Layton wrote:
> On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote:
> > Introduce a mount context concept.
trivia:
> > static int selinux_mount_ctx_option(struct mount_context *mc, char *opt)
> > +{
[]
> > + if (opts->mnt_opts) {
> > + oo = kmalloc((opts->num_mnt_opts + 1) * sizeof(char *),
> > + GFP_KERNEL);
> > + if (!oo)
> > + return -ENOMEM;
> > + memcpy(oo, opts->mnt_opts, opts->num_mnt_opts * sizeof(char *));
> > + oo[opts->num_mnt_opts] = NULL;
> > + old = opts->mnt_opts;
> > + opts->mnt_opts = oo;
> > + kfree(old);
> > + }
krealloc would probably be more efficient and possible
readable as likely there's already padding in the original
allocation.
Are there no locking constraints?
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-03 20:40 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD7lT-DA-17@gated-at.bofh.it> |
| In reply to | #1635194 |
Joe Perches <joe@perches.com> wrote:
> krealloc would probably be more efficient and possible
> readable as likely there's already padding in the original
> allocation.
The problem is if krealloc() fails: you've lost all those pointers to things
you then need to free.
> Are there no locking constraints?
Generally, no, not until you do the ->mount() op. Also remounting needs a
lock, but that's already done with the sb->s_umount lock.
However, that said, if you do:
fd = fsopen("foofs");
write(fd, "o foo=bar", ...);
fsmount(fd, "/foo");
then the fsmount() and write() calls have to lock against other fsmount() and
write() calls. I use the inode lock for this. [Note that it probably should
be interruptible rather than just killable, but there's no primitive for that
as yet].
David
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-05-03 20:50 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD7vA-Hg-21@gated-at.bofh.it> |
| In reply to | #1635199 |
On Wed, 2017-05-03 at 19:37 +0100, David Howells wrote:
> Joe Perches <joe@perches.com> wrote:
>
> > krealloc would probably be more efficient and possible
> > readable as likely there's already padding in the original
> > allocation.
>
> The problem is if krealloc() fails: you've lost all those pointers to things
> you then need to free.
Huh? How could that happen?
krealloc must always use a temporary.
If krealloc returns NULL, the original allocation is kept.
> > Are there no locking constraints?
>
> Generally, no, not until you do the ->mount() op. Also remounting needs a
> lock, but that's already done with the sb->s_umount lock.
>
> However, that said, if you do:
>
> fd = fsopen("foofs");
> write(fd, "o foo=bar", ...);
> fsmount(fd, "/foo");
>
> then the fsmount() and write() calls have to lock against other fsmount() and
> write() calls. I use the inode lock for this. [Note that it probably should
> be interruptible rather than just killable, but there's no primitive for that
> as yet].
>
> David
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-03 22:20 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD8UF-1LK-17@gated-at.bofh.it> |
| In reply to | #1635210 |
Joe Perches <joe@perches.com> wrote: > > > krealloc would probably be more efficient and possible > > > readable as likely there's already padding in the original > > > allocation. > > > > The problem is if krealloc() fails: you've lost all those pointers to things > > you then need to free. > > Huh? How could that happen? > > krealloc must always use a temporary. > If krealloc returns NULL, the original allocation is kept. Hmmm... Good point. David
[toc] | [prev] | [next] | [standalone]
| From | Matthew Wilcox <willy@infradead.org> |
|---|---|
| Date | 2017-05-03 22:40 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD9e2-1So-17@gated-at.bofh.it> |
| In reply to | #1635194 |
On Wed, May 03, 2017 at 11:26:38AM -0700, Joe Perches wrote: > On Wed, 2017-05-03 at 14:13 -0400, Jeff Layton wrote: > > On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote: > > > + oo = kmalloc((opts->num_mnt_opts + 1) * sizeof(char *), > > > + GFP_KERNEL); If we're picking nits, then this should be kcalloc in case somebody passed in 2^31 in num_mnt_opts.
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-03 23:20 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tD9QK-2oD-3@gated-at.bofh.it> |
| In reply to | #1635266 |
Matthew Wilcox <willy@infradead.org> wrote:
> On Wed, May 03, 2017 at 11:26:38AM -0700, Joe Perches wrote:
> > On Wed, 2017-05-03 at 14:13 -0400, Jeff Layton wrote:
> > > On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote:
> > > > + oo = kmalloc((opts->num_mnt_opts + 1) * sizeof(char *),
> > > > + GFP_KERNEL);
>
> If we're picking nits, then this should be kcalloc in case somebody
> passed in 2^31 in num_mnt_opts.
A few lines previously there is:
if (opts->num_mnt_opts > 3) {
mc->error = "SELinux: Too many options";
return -EINVAL;
}
David
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-05-03 23:40 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDaa5-2vi-3@gated-at.bofh.it> |
| In reply to | #1635266 |
(adding Julia Lawall and cocci)
On Wed, 2017-05-03 at 13:38 -0700, Matthew Wilcox wrote:
> On Wed, May 03, 2017 at 11:26:38AM -0700, Joe Perches wrote:
> > On Wed, 2017-05-03 at 14:13 -0400, Jeff Layton wrote:
> > > On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote:
> > > > + oo = kmalloc((opts->num_mnt_opts + 1) * sizeof(char *),
> > > > + GFP_KERNEL);
>
> If we're picking nits, then this should be kcalloc in case somebody
> passed in 2^31 in num_mnt_opts.
There are likely dozens to hundreds of possible/silent
multiplication overflow defects in the kernel, not just
in allocations.
Auditing the sources would seem labor intensive.
Perhaps coccinelle could help find them.
Perhaps there should be some overflow checking functions
added to math64.h
Maybe some form like:
u32 u32_mul_u32_u32(u32 a, u32 b)
{
u32 res = a * b;
WARN_ON(a != 0 && res / a != b);
return res;
}
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2017-05-04 08:30 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDiqZ-8aF-1@gated-at.bofh.it> |
| In reply to | #1635286 |
[Multipart message — attachments visible in raw view] — view raw
On Wed, 3 May 2017, Joe Perches wrote:
> (adding Julia Lawall and cocci)
>
> On Wed, 2017-05-03 at 13:38 -0700, Matthew Wilcox wrote:
> > On Wed, May 03, 2017 at 11:26:38AM -0700, Joe Perches wrote:
> > > On Wed, 2017-05-03 at 14:13 -0400, Jeff Layton wrote:
> > > > On Wed, 2017-05-03 at 17:04 +0100, David Howells wrote:
> > > > > + oo = kmalloc((opts->num_mnt_opts + 1) * sizeof(char *),
> > > > > + GFP_KERNEL);
> >
> > If we're picking nits, then this should be kcalloc in case somebody
> > passed in 2^31 in num_mnt_opts.
>
> There are likely dozens to hundreds of possible/silent
> multiplication overflow defects in the kernel, not just
> in allocations.
>
> Auditing the sources would seem labor intensive.
>
> Perhaps coccinelle could help find them.
>
> Perhaps there should be some overflow checking functions
> added to math64.h
>
> Maybe some form like:
>
> u32 u32_mul_u32_u32(u32 a, u32 b)
> {
> u32 res = a * b;
>
> WARN_ON(a != 0 && res / a != b);
>
> return res;
> }
Coccinelle doesn't kow about the values of variables. It would need some
heuristics about where potentially large values can come from.
julia
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-04 11:30 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDlfd-1wD-49@gated-at.bofh.it> |
| In reply to | #1635194 |
Joe Perches <joe@perches.com> wrote: > krealloc would probably be more efficient and possible > readable as likely there's already padding in the original > allocation. Given there's a maximum of 3 slots, I think it makes better sense to just allocate them all up front. David
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2017-05-04 16:40 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDq5b-4H2-1@gated-at.bofh.it> |
| In reply to | #1635586 |
On Thu, 2017-05-04 at 10:27 +0100, David Howells wrote: > Joe Perches <joe@perches.com> wrote: > > > krealloc would probably be more efficient and possible > > readable as likely there's already padding in the original > > allocation. > > Given there's a maximum of 3 slots, I think it makes better sense to just > allocate them all up front. Sounds good to me. Simpler is frequently better.
[toc] | [prev] | [next] | [standalone]
| From | Rasmus Villemoes <linux@rasmusvillemoes.dk> |
|---|---|
| Date | 2017-05-03 23:50 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDajL-2yX-1@gated-at.bofh.it> |
| In reply to | #1635100 |
On Wed, May 03 2017, David Howells <dhowells@redhat.com> wrote:
> fs_type->fsopen() is called to set it up. fs_type->mc_size says how much
> should be added on to the mount context for the filesystem's use.
This is repeated several times in the documentation, but the code says
that ->mc_size should be the full size of the struct wrapping struct
mount_context.
> diff --git a/fs/mount.h b/fs/mount.h
> index 2826543a131d..b1e99b38f2ee 100644
> --- a/fs/mount.h
> +++ b/fs/mount.h
> @@ -108,9 +108,10 @@ static inline void detach_mounts(struct dentry *dentry)
> __detach_mounts(dentry);
> }
>
> -static inline void get_mnt_ns(struct mnt_namespace *ns)
> +static inline struct mnt_namespace *get_mnt_ns(struct mnt_namespace *ns)
> {
> atomic_inc(&ns->count);
> + return ns;
> }
>
> extern seqlock_t mount_lock;
it's not much, but at least this could go into a patch of its own.
> +/**
> + * __vfs_fsopen - Open a filesystem and create a mount context
> + * @fs_type: The filesystem type
> + * @src_sb: A superblock from which this one derives (or NULL)
> + * @ms_flags: Superblock flags and op flags (such as MS_REMOUNT)
> + * @mnt_flags: Mountpoint flags, such as MNT_READONLY
> + * @mount_type: Type of mount
> + *
> + * Open a filesystem and create a mount context. The mount context is
> + * initialised with the supplied flags and, if a submount/automount from
> + * another superblock (@src_sb), may have parameters such as namespaces copied
> + * across from that superblock.
> + */
> +struct mount_context *__vfs_fsopen(struct file_system_type *fs_type,
> + struct super_block *src_sb,
> + unsigned int ms_flags, unsigned int mnt_flags,
> + enum mount_type mount_type)
> +{
> + struct mount_context *mc;
> + int ret;
> +
> + if (fs_type->fsopen && fs_type->mc_size < sizeof(*mc))
> + BUG();
So ->mc_size can be 0 (i.e. not explicitly initialized) if fs_type does
not have ->fsopen. OK.
> + mc = kzalloc(max_t(size_t, fs_type->mc_size, sizeof(*mc)), GFP_KERNEL);
In which case we round up to sizeof(*mc). OK.
> + if (!mc)
> + return ERR_PTR(-ENOMEM);
> +
> + mc->mount_type = mount_type;
> + mc->ms_flags = ms_flags;
> + mc->mnt_flags = mnt_flags;
> + mc->fs_type = fs_type;
> + get_filesystem(fs_type);
Maybe get_filesystem should also be taught to return its argument so
this could be written like the below assignments.
> + mc->mnt_ns = get_mnt_ns(current->nsproxy->mnt_ns);
> + mc->pid_ns = get_pid_ns(task_active_pid_ns(current));
> + mc->net_ns = get_net(current->nsproxy->net_ns);
> + mc->user_ns = get_user_ns(current_user_ns());
> + mc->cred = get_current_cred();
> +
> +
> +/**
> + * vfs_dup_mount_context: Duplicate a mount context.
> + * @src: The mount context to copy.
> + */
> +struct mount_context *vfs_dup_mount_context(struct mount_context *src)
> +{
> + struct mount_context *mc;
> + int ret;
> +
> + if (!src->ops->dup)
> + return ERR_PTR(-ENOTSUPP);
> +
> + mc = kmemdup(src, src->fs_type->mc_size, GFP_KERNEL);
So this assumes that vfs_dup_mount_context is only used if ->mc_size is
explicitly initialized. A max_t here as well probably wouldn't hurt.
> + unsigned short mc_size; /* Size of mount context to allocate */
Any particular reason to use a short? The struct doesn't pack any better.
> +static int selinux_mount_ctx_dup(struct mount_context *mc,
> + struct mount_context *src_mc)
> +{
> + const struct security_mnt_opts *src = src_mc->security;
> + struct security_mnt_opts *opts;
> + int i, n;
> +
> + opts = kzalloc(sizeof(*opts), GFP_KERNEL);
> + if (!opts)
> + return -ENOMEM;
> + mc->security = opts;
> +
> + if (!src || !src->num_mnt_opts)
> + return 0;
> + n = opts->num_mnt_opts = src->num_mnt_opts;
> +
> + if (opts->mnt_opts) {
should probably be src->mnt_opts
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-04 12:30 +0200 |
| Subject | Re: [PATCH 3/9] VFS: Introduce a mount context |
| Message-ID | <tDmbf-28q-9@gated-at.bofh.it> |
| In reply to | #1635290 |
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote: > > + if (fs_type->fsopen && fs_type->mc_size < sizeof(*mc)) > > + BUG(); > > So ->mc_size can be 0 (i.e. not explicitly initialized) if fs_type does > not have ->fsopen. OK. I need to be able to handle filesystems that don't support this yet. Once all filesystems support this, I would be able to take away the max_t() thing. > > + if (!src->ops->dup) > > + return ERR_PTR(-ENOTSUPP); > > + > > + mc = kmemdup(src, src->fs_type->mc_size, GFP_KERNEL); > > So this assumes that vfs_dup_mount_context is only used if ->mc_size is > explicitly initialized. A max_t here as well probably wouldn't hurt. If you don't provide an ->fsopen() op, you can't set src->ops, you don't see a mount context and you can't call this function. If you did supply an ->fsopen() op, the BUG() would've got you if you didn't set ->mc_size. > > + unsigned short mc_size; /* Size of mount context to allocate */ > > Any particular reason to use a short? The struct doesn't pack any better. But it leaves a hole someone else can use. I try not to use fields larger than I need to. David
[toc] | [prev] | [next] | [standalone]
| From | Miklos Szeredi <mszeredi@redhat.com> |
|---|---|
| Date | 2017-05-05 16:40 +0200 |
| Message-ID | <tDMyJ-2YY-7@gated-at.bofh.it> |
| In reply to | #1635100 |
On Wed, May 3, 2017 at 6:04 PM, David Howells <dhowells@redhat.com> wrote:
>
> Here are a set of patches to create a mount context prior to setting up a
> new mount, populating it with the parsed options/binary data and then
> effecting the mount.
Great work, thanks for taking this on.
I'd argue with some design decisions here. One of the motivations for
doing the mount API overhaul is to create clear distinction between
separate functions like:
- creating filesystem instance (aka superblock)
- attaching filesystem instance into mount tree
- reconfiguring superblock
- changing mount properties
This patchset achieves this partly, but the separation is far from
crisp clear... First of all why is fsopen() creating a "mount
context"? It's suppsed to create a "superblock creation context".
And indeed, there are mount flags and root path in there, which are
definitely not necessary for creating a super block.
Is there a good reason why these mount specific properties leaked into
the object created by fsopen()?
Also I'd expect all context ops to be fully generic first. I.e. no
filesystem code needs to be touched to make the new interface work.
The context would just build the option string and when everything is
ready (probably need a "commit" command) then it would go off and call
mount_fs() to create the superblock and attach it to the context.
Then, when that works, we could add context ops, so the filesystem can
do various things along the way, which is the other reason we want
this. And in the end it would allow gradual migration to a new
superblock creation api and phasing out the old one. But that
shouldn't be observable on either the old or the new userspace
interfaces.
> 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.
>
> Further developments:
>
> (*) 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.
First we can limit this feature to non-weird (ie. no managed dentries) subtrees.
>
> (*) Move the pid_ns pointer from struct mount_context to struct
> proc_mount_context as I'm not sure it's necessary for anything other
> than procfs.
>
> (*) 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
Will try to review the actual patches next week.
Thanks,
Miklos
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-05 17:50 +0200 |
| Message-ID | <tDNEt-3DR-9@gated-at.bofh.it> |
| In reply to | #1636427 |
Miklos Szeredi <mszeredi@redhat.com> wrote: > I'd argue with some design decisions here. One of the motivations for > doing the mount API overhaul is to create clear distinction between > separate functions like: > > - creating filesystem instance (aka superblock) > > - attaching filesystem instance into mount tree > > - reconfiguring superblock > > - changing mount properties I definitely agree that keeping a separation between vfsmount manipulation (add, bind, move, ...) and superblock manipulation (create, remount) is a good idea. However, creating new superblocks and remounting superblocks have a lot in common, including the option parsing. Note also that existing code is somewhat lazy about rejecting parameters that can't be changed with a remount and will ignore some attempted changes. We have to retain this behaviour, at least for the normal mount() system call. Note that one of the main reasons I'm working on this is namespace propagation, particularly with respect to automounts. > This patchset achieves this partly, but the separation is far from > crisp clear... First of all why is fsopen() creating a "mount > context"? It's suppsed to create a "superblock creation context". I've no particular objection to renaming struct mount_context to something else, but it also needs to handle remount because of the commonality. Further, once you've created a superblock, what are you going to do with it other than mount it? I suppose you could statfs it and we could add other superblock manipulation functions, but this is normally done by opening the device directly (at least for bdev-based superblocks). > And indeed, there are mount flags and root path in there, which are > definitely not necessary for creating a super block. Erm, that's not strictly true. Some filesystems (eg. nfs, ocfs2, lustre) want to know about certain MNT_xxx flags, such as MNT_NOATIME and MNT_READONLY. Further, the root path might be necessary for the mount - see NFS for example. What I was thinking of, say for NFS, is splitting the source name up front, so: my.nfs.org:/my/home/dir into: mc->device = "my.nfs.org"; mc->root_path = "/my/home/dir"; and then having the VFS handle the root walk rather than doing it inside NFS. This facility could then become available to other filesystems potentially. However, with the case on NFS, you may need to hand the root path off to a mount server. > Is there a good reason why these mount specific properties leaked into > the object created by fsopen()? Answered above. I'm okay with removing remove root_path from the context for the moment. It's something that can be revisited later. We also might need to remove usage of MNT_xxx flags from filesystems. > Also I'd expect all context ops to be fully generic first. I.e. no > filesystem code needs to be touched to make the new interface work. > The context would just build the option string and when everything is > ready (probably need a "commit" command) then it would go off and call > mount_fs() to create the superblock and attach it to the context. That should be easy enough to add as a fallback. > Then, when that works, we could add context ops, so the filesystem can > do various things along the way, which is the other reason we want > this. And in the end it would allow gradual migration to a new > superblock creation api and phasing out the old one. I'm not sure the context ops are so easily to add gradually. > But that shouldn't be observable on either the old or the new userspace > interfaces. Almost a fair point - but it can be observed by pushing in more than a page's worth of options. What I have now for NFS will still work with fsopen()/write()/fsmount() whereas mount() won't. David
[toc] | [prev] | [next] | [standalone]
| From | Miklos Szeredi <mszeredi@redhat.com> |
|---|---|
| Date | 2017-05-08 10:30 +0200 |
| Message-ID | <tEMdk-1t2-11@gated-at.bofh.it> |
| In reply to | #1636464 |
On Fri, May 5, 2017 at 5:47 PM, David Howells <dhowells@redhat.com> wrote: > Miklos Szeredi <mszeredi@redhat.com> wrote: > >> I'd argue with some design decisions here. One of the motivations for >> doing the mount API overhaul is to create clear distinction between >> separate functions like: >> >> - creating filesystem instance (aka superblock) >> >> - attaching filesystem instance into mount tree >> >> - reconfiguring superblock >> >> - changing mount properties > > I definitely agree that keeping a separation between vfsmount manipulation > (add, bind, move, ...) and superblock manipulation (create, remount) is a good > idea. > > However, creating new superblocks and remounting superblocks have a lot in > common, including the option parsing. Note also that existing code is > somewhat lazy about rejecting parameters that can't be changed with a remount > and will ignore some attempted changes. We have to retain this behaviour, at > least for the normal mount() system call. > > Note that one of the main reasons I'm working on this is namespace > propagation, particularly with respect to automounts. > >> This patchset achieves this partly, but the separation is far from >> crisp clear... First of all why is fsopen() creating a "mount >> context"? It's suppsed to create a "superblock creation context". > > I've no particular objection to renaming struct mount_context to something > else, but it also needs to handle remount because of the commonality. Definitely agree about having the same object handle filesystem creation and filesystem reconfiguration. And yeah, I think naming does have to be changed, simply because the old way of doing things is so ingrained in everything in this area. > Further, once you've created a superblock, what are you going to do with it > other than mount it? I suppose you could statfs it and we could add other > superblock manipulation functions, but this is normally done by opening the > device directly (at least for bdev-based superblocks). It surely makes sense to mount it, but that does not mean we need to blur the boundary. >> And indeed, there are mount flags and root path in there, which are >> definitely not necessary for creating a super block. > > Erm, that's not strictly true. > > Some filesystems (eg. nfs, ocfs2, lustre) want to know about certain MNT_xxx > flags, such as MNT_NOATIME and MNT_READONLY. So sometimes filesystems have access to mount flags that tell about the mount that the operation was done through (the only relevant op is getattr(), right?). But that doesn't mean the filesystem has any business looking at the mount flags that the initial mount will be created with (it definitely does not). > Further, the root path might be necessary for the mount - see NFS for example. > What I was thinking of, say for NFS, is splitting the source name up front, > so: > > my.nfs.org:/my/home/dir > > into: > > mc->device = "my.nfs.org"; > mc->root_path = "/my/home/dir"; > > and then having the VFS handle the root walk rather than doing it inside NFS. > This facility could then become available to other filesystems potentially. Ah, I see what you are saying: we don't have the infrastructure currently in the VFS to handle NFS subdir mounts, but want to introduce this in the interface now (because NFS *can* handle it), so that when we will have the VFS infrastructure we can silently switch to it. > However, with the case on NFS, you may need to hand the root path off to a > mount server. Hmm... IMO the right way to do this would be to move the NFS subdir code first to the VFS and then introduce the new interface with the correct semantics (i.e. subdir is handled on mount not on superblock creation). I haven't looked at what NFS is doing, but subdir mounts should be just like plain bind mounts, no? The special thing here is that we need to do that bind mount before the filesystem is initially mounted. But that can be done by - first doing an kernel-internal mount - doing the pathwalk on that (which can trigger more internal mounts) - attaching the found source to the target mountpoint - finally discarding the kernel internal mount tree >> Is there a good reason why these mount specific properties leaked into >> the object created by fsopen()? > > Answered above. I'm okay with removing remove root_path from the context for > the moment. It's something that can be revisited later. > > We also might need to remove usage of MNT_xxx flags from filesystems. There is no MNT_xxx flag usage in superblock creation, because mnt_flags are not passed to filesystems and the relevant MS_xxx ones are cleared from the flags that *are* passed to the fs. MS_RDONLY is special, because it's a mount flag as well as a superblock flag. No problem there, with the new interface it will be possible to set them separately (e.g. ro on sb and rw on mnt or vice versa). >> But that shouldn't be observable on either the old or the new userspace >> interfaces. > > Almost a fair point - but it can be observed by pushing in more than a page's > worth of options. What I have now for NFS will still work with > fsopen()/write()/fsmount() whereas mount() won't. Alright, lets just stay, that everything that works now should work the same way on the old as well as the new interfaces. Thanks, Miklos
[toc] | [prev] | [next] | [standalone]
| From | David Howells <dhowells@redhat.com> |
|---|---|
| Date | 2017-05-08 10:40 +0200 |
| Message-ID | <tEMmZ-1w4-1@gated-at.bofh.it> |
| In reply to | #1636464 |
David Howells <dhowells@redhat.com> wrote: > > This patchset achieves this partly, but the separation is far from > > crisp clear... First of all why is fsopen() creating a "mount > > context"? It's suppsed to create a "superblock creation context". > > I've no particular objection to renaming struct mount_context to something > else, but it also needs to handle remount because of the commonality. > > Further, once you've created a superblock, what are you going to do with it > other than mount it? I suppose you could statfs it and we could add other > superblock manipulation functions, but this is normally done by opening the > device directly (at least for bdev-based superblocks). How about sb_context, sb_config, sb_parameters or something like that? David
[toc] | [prev] | [next] | [standalone]
Page 2 of 4 — ← Prev page 1 [2] 3 4 Next page →
Back to top | Article view | linux.kernel
csiph-web