linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alistair Strachan <astrachan@google.com>
To: linux-fsdevel@vger.kernel.org
Cc: Alistair Strachan <astrachan@google.com>,
	Seth Forshee <seth.forshee@canonical.com>,
	Djalal Harouni <tixxdz@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	kernel-team@android.com, linux-kernel@vger.kernel.org
Subject: [PATCH] proc: Fix parsing of mount parameters.
Date: Mon, 11 Jun 2018 12:57:44 -0700	[thread overview]
Message-ID: <20180611195744.154962-1-astrachan@google.com> (raw)

In commit e94591d0d90c "proc: Convert proc_mount to use mount_ns"
the parsing of mount parameters for the proc filesystem was broken.

The SB_KERNMOUNT for procfs happens via:

  start_kernel()
    rest_init()
      kernel_thread()
        _do_fork()
           copy_process()
             alloc_pid()
               pid_ns_prepare_proc()
                 kern_mount_data()
                   proc_mount()
                     mount_ns()

In mount_ns(), the kernel calls proc_fill_super() only if the superblock
has not previously been set up (i.e. the first mount reference),
regardless of SB_KERNMOUNT. Because the call to proc_parse_options() had
been moved inside here, and the SB_KERNMOUNT uses no mount options, the
option parser became a no-op.

When userspace later mounted procfs with e.g. hidepid=2, the options
would be ignored.

This change backs out a part of the original cleanup and parses the
procfs mount options at every mount call. Because the options currently
only update the pid_ns for the mount, they are applied for all mounts of
proc by that pid or childen of that pid, instantaneously. This is the
same behavior as the original code.

Fixes: e94591d0d90c ("proc: Convert proc_mount to use mount_ns")
Signed-off-by: Alistair Strachan <astrachan@google.com>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Djalal Harouni <tixxdz@gmail.com>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: kernel-team@android.com
Cc: linux-kernel@vger.kernel.org
---
 fs/proc/inode.c    | 4 ----
 fs/proc/internal.h | 1 -
 fs/proc/root.c     | 5 ++++-
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 2cf3b74391ca..bbbbf348be0a 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -492,13 +492,9 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
 
 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 |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 50cb22a08c2f..89b7e845b000 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -264,7 +264,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 61b7340b357a..d40676a5dd6c 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -36,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];
@@ -98,6 +98,9 @@ static struct dentry *proc_mount(struct file_system_type *fs_type,
 		ns = task_active_pid_ns(current);
 	}
 
+	if (!proc_parse_options(data, ns))
+		return ERR_PTR(-EINVAL);
+
 	return mount_ns(fs_type, flags, data, ns, ns->user_ns, proc_fill_super);
 }
 
-- 
2.18.0.rc1.242.g61856ae69a-goog


             reply	other threads:[~2018-06-11 19:58 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-11 19:57 Alistair Strachan [this message]
2018-06-12  1:22 ` [PATCH] proc: Fix parsing of mount parameters Eric W. Biederman
2018-06-12  6:12   ` Alistair Strachan
2018-06-12 14:59     ` Eric W. Biederman
2018-06-16  3:26       ` [CFT][PATCH] proc: Simplify and fix proc by removing the kernel mount Eric W. Biederman
2018-06-17  2:54         ` [PATCH v2] " Eric W. Biederman
2018-06-17  6:20           ` Alistair Strachan

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=20180611195744.154962-1-astrachan@google.com \
    --to=astrachan@google.com \
    --cc=ebiederm@xmission.com \
    --cc=kernel-team@android.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=seth.forshee@canonical.com \
    --cc=tixxdz@gmail.com \
    /path/to/YOUR_REPLY

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

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).