linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Calvin Owens <calvinowens@fb.com>
To: Cyrill Gorcunov <gorcunov@gmail.com>,
	"Kirill A. Shutemov" <kirill@shutemov.name>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
	Peter Feiner <pfeiner@google.com>,
	Grant Likely <grant.likely@secretlab.ca>,
	Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com>,
	<linux-kernel@vger.kernel.org>, <kernel-team@fb.com>,
	Pavel Emelyanov <xemul@openvz.org>
Subject: [RFC][PATCH v3] procfs: Always expose /proc/<pid>/map_files/ and make it readable
Date: Wed, 11 Feb 2015 18:29:10 -0800	[thread overview]
Message-ID: <20150212022910.GA3247638@mail.thefacebook.com> (raw)
In-Reply-To: <20150124031544.GA1992748@mail.thefacebook.com>

Currently, /proc/<pid>/map_files/ is restricted to CAP_SYS_ADMIN, and is
only exposed if CONFIG_CHECKPOINT_RESTORE is set. This interface is
useful for enumerating the files mapped into a process when the more
verbose information in /proc/<pid>/maps is not needed. It also allows
access to file descriptors for files that have been deleted and closed
but are still mmapped into a process, which can be very useful for
introspection and debugging.

This patch moves the folder out from behind CHECKPOINT_RESTORE, and
removes the CAP_SYS_ADMIN restrictions. With that change alone,
accessing this interface would have required PTRACE_MODE_READ like the
links in /proc/<pid>/fd/*.

However, a discussion on lkml concluded that MODE_READ is not
sufficient, both because write access to the inodes these links point
to allows direct modification of a process's address space, and
because it exposes files that users may have overlooked permissions on
because it was assumed they would be inaccessible (either deleted as
per above, or created via O_TMPFILE).

So, in addition to the above, this patch enforces PTRACE_MODE_ATTACH on
all the map_files/ operations. Since this is the same check that
determines if access to /proc/<pid>/mem is allowed, it will not allow an
attacker to do anything that was not already possible through that
interface.

Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
Changes in v3:	Changed permission checks to use PTRACE_MODE_ATTACH
		instead of PTRACE_MODE_READ, and added a stub to
		enforce MODE_ATTACH on follow_link() as well.

Changes in v2:	Removed the follow_link() stub that returned -EPERM if
		the caller didn't have CAP_SYS_ADMIN, since the caller
		in my chroot() scenario gets -EACCES anyway.

 fs/proc/base.c | 59 ++++++++++++++++++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 3f3d7ae..1355a4d 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1632,8 +1632,6 @@ end_instantiate:
 	return dir_emit(ctx, name, len, 1, DT_UNKNOWN);
 }
 
-#ifdef CONFIG_CHECKPOINT_RESTORE
-
 /*
  * dname_to_vma_addr - maps a dentry name into two unsigned longs
  * which represent vma start and end addresses.
@@ -1660,17 +1658,12 @@ static int map_files_d_revalidate(struct dentry *dentry, unsigned int flags)
 	if (flags & LOOKUP_RCU)
 		return -ECHILD;
 
-	if (!capable(CAP_SYS_ADMIN)) {
-		status = -EPERM;
-		goto out_notask;
-	}
-
 	inode = dentry->d_inode;
 	task = get_proc_task(inode);
 	if (!task)
 		goto out_notask;
 
-	mm = mm_access(task, PTRACE_MODE_READ);
+	mm = mm_access(task, PTRACE_MODE_ATTACH);
 	if (IS_ERR_OR_NULL(mm))
 		goto out;
 
@@ -1753,6 +1746,39 @@ struct map_files_info {
 	unsigned char	name[4*sizeof(long)+2]; /* max: %lx-%lx\0 */
 };
 
+/*
+ * Enforce stronger PTRACE_MODE_ATTACH permissions on the symlinks under
+ * /proc/<pid>/map_files, since these links may refer to deleted or O_TMPFILE
+ * files that users might assume are inaccessible regardless of their
+ * ownership/permissions.
+ */
+static void *proc_map_files_follow_link(struct dentry *dentry, struct nameidata *nd)
+{
+	struct inode *inode = dentry->d_inode;
+	struct task_struct *task;
+	int allowed = 0;
+
+	task = get_proc_task(inode);
+	if (task) {
+		allowed = ptrace_may_access(task, PTRACE_MODE_ATTACH);
+		put_task_struct(task);
+	}
+
+	if (!allowed)
+		return ERR_PTR(-EACCES);
+
+	return proc_pid_follow_link(dentry, nd);
+}
+
+/*
+ * Identical to proc_pid_link_inode_operations except for follow_link()
+ */
+static const struct inode_operations proc_map_files_link_inode_operations = {
+	.readlink	= proc_pid_readlink,
+	.follow_link	= proc_map_files_follow_link,
+	.setattr	= proc_setattr,
+};
+
 static int
 proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
 			   struct task_struct *task, const void *ptr)
@@ -1768,7 +1794,7 @@ proc_map_files_instantiate(struct inode *dir, struct dentry *dentry,
 	ei = PROC_I(inode);
 	ei->op.proc_get_link = proc_map_files_get_link;
 
-	inode->i_op = &proc_pid_link_inode_operations;
+	inode->i_op = &proc_map_files_link_inode_operations;
 	inode->i_size = 64;
 	inode->i_mode = S_IFLNK;
 
@@ -1792,17 +1818,13 @@ static struct dentry *proc_map_files_lookup(struct inode *dir,
 	int result;
 	struct mm_struct *mm;
 
-	result = -EPERM;
-	if (!capable(CAP_SYS_ADMIN))
-		goto out;
-
 	result = -ENOENT;
 	task = get_proc_task(dir);
 	if (!task)
 		goto out;
 
 	result = -EACCES;
-	if (!ptrace_may_access(task, PTRACE_MODE_READ))
+	if (!ptrace_may_access(task, PTRACE_MODE_ATTACH))
 		goto out_put_task;
 
 	result = -ENOENT;
@@ -1849,17 +1871,13 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx)
 	struct map_files_info *p;
 	int ret;
 
-	ret = -EPERM;
-	if (!capable(CAP_SYS_ADMIN))
-		goto out;
-
 	ret = -ENOENT;
 	task = get_proc_task(file_inode(file));
 	if (!task)
 		goto out;
 
 	ret = -EACCES;
-	if (!ptrace_may_access(task, PTRACE_MODE_READ))
+	if (!ptrace_may_access(task, PTRACE_MODE_ATTACH))
 		goto out_put_task;
 
 	ret = 0;
@@ -2040,7 +2058,6 @@ static const struct file_operations proc_timers_operations = {
 	.llseek		= seq_lseek,
 	.release	= seq_release_private,
 };
-#endif /* CONFIG_CHECKPOINT_RESTORE */
 
 static int proc_pident_instantiate(struct inode *dir,
 	struct dentry *dentry, struct task_struct *task, const void *ptr)
@@ -2537,9 +2554,7 @@ static const struct inode_operations proc_task_inode_operations;
 static const struct pid_entry tgid_base_stuff[] = {
 	DIR("task",       S_IRUGO|S_IXUGO, proc_task_inode_operations, proc_task_operations),
 	DIR("fd",         S_IRUSR|S_IXUSR, proc_fd_inode_operations, proc_fd_operations),
-#ifdef CONFIG_CHECKPOINT_RESTORE
 	DIR("map_files",  S_IRUSR|S_IXUSR, proc_map_files_inode_operations, proc_map_files_operations),
-#endif
 	DIR("fdinfo",     S_IRUSR|S_IXUSR, proc_fdinfo_inode_operations, proc_fdinfo_operations),
 	DIR("ns",	  S_IRUSR|S_IXUGO, proc_ns_dir_inode_operations, proc_ns_dir_operations),
 #ifdef CONFIG_NET
-- 
1.8.1

  parent reply	other threads:[~2015-02-12  2:29 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-14  0:20 [RFC][PATCH] procfs: Add /proc/<pid>/mapped_files Calvin Owens
2015-01-14  0:23 ` Calvin Owens
2015-01-14 14:13 ` Rasmus Villemoes
2015-01-14 14:37   ` Siddhesh Poyarekar
2015-01-14 14:53     ` Rasmus Villemoes
2015-01-14 21:03       ` Calvin Owens
2015-01-14 22:45         ` Andrew Morton
2015-01-14 23:51           ` Rasmus Villemoes
2015-01-16  1:15             ` Andrew Morton
2015-01-16 11:00               ` Kirill A. Shutemov
2015-01-14 15:25 ` Kirill A. Shutemov
2015-01-14 15:33   ` Cyrill Gorcunov
2015-01-14 20:46     ` Calvin Owens
2015-01-14 21:16       ` Cyrill Gorcunov
2015-01-22  2:45         ` [RFC][PATCH] procfs: Always expose /proc/<pid>/map_files/ and make it readable Calvin Owens
2015-01-22  7:16           ` Cyrill Gorcunov
2015-01-22 11:02           ` Kirill A. Shutemov
2015-01-22 21:00             ` Calvin Owens
2015-01-22 21:27               ` Kirill A. Shutemov
2015-01-23  5:52                 ` Calvin Owens
2015-01-24  3:15           ` [RFC][PATCH v2] " Calvin Owens
2015-01-26 12:47             ` Kirill A. Shutemov
2015-01-26 21:00               ` Cyrill Gorcunov
2015-01-26 23:43                 ` Andrew Morton
2015-01-27  0:15                   ` Kees Cook
2015-01-27  7:37                     ` Cyrill Gorcunov
2015-01-27 19:53                       ` Kees Cook
2015-01-27 21:35                         ` Cyrill Gorcunov
2015-01-27 21:46                         ` Pavel Emelyanov
2015-01-27  0:19                   ` Kirill A. Shutemov
2015-01-27  6:46                   ` Cyrill Gorcunov
2015-01-27  6:50                     ` Andrew Morton
2015-01-27  7:23                       ` Cyrill Gorcunov
2015-01-28  4:38                   ` Calvin Owens
2015-01-30  1:30                     ` Kees Cook
2015-01-31  1:58                       ` Calvin Owens
2015-02-02 14:01                         ` Austin S Hemmelgarn
2015-02-04  3:53                           ` Calvin Owens
2015-02-02 20:16                         ` Andy Lutomirski
2015-02-04  3:28                           ` Calvin Owens
2015-02-12  2:29             ` Calvin Owens [this message]
2015-02-12  7:45               ` [RFC][PATCH v3] " Cyrill Gorcunov
2015-02-14 20:40               ` [RFC][PATCH v4] " Calvin Owens
2015-03-10 22:17                 ` Cyrill Gorcunov
2015-04-28 22:23                   ` Calvin Owens
2015-04-29  7:32                     ` Cyrill Gorcunov
2015-05-19  3:10                 ` [PATCH v5] " Calvin Owens
2015-05-19  3:29                   ` Joe Perches
2015-05-19 18:04                   ` Andy Lutomirski
2015-05-21  1:52                     ` Calvin Owens
2015-05-21  2:10                       ` Andy Lutomirski
2015-06-09  3:39                   ` [PATCH v6] " Calvin Owens
2015-06-09 17:27                     ` Kees Cook
2015-06-09 17:47                       ` Andy Lutomirski
2015-06-09 18:15                         ` Cyrill Gorcunov
2015-06-09 21:13                     ` Andrew Morton
2015-06-10  1:39                       ` Calvin Owens
2015-06-10 20:58                         ` Andrew Morton
2015-06-11 11:10                           ` Alexey Dobriyan
2015-06-11 18:49                             ` Andrew Morton
2015-06-12  9:55                               ` Alexey Dobriyan
2015-06-19  2:32                     ` [PATCH v7] " Calvin Owens
2015-07-15 22:21                       ` Andrew Morton
2015-07-15 23:39                         ` Calvin Owens
2015-02-14 20:44               ` [PATCH] procfs: Return -ESRCH on /proc/N/fd/* when PID N doesn't exist Calvin Owens
2015-01-14 22:40 ` [RFC][PATCH] procfs: Add /proc/<pid>/mapped_files Kirill A. Shutemov

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=20150212022910.GA3247638@mail.thefacebook.com \
    --to=calvinowens@fb.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=gorcunov@gmail.com \
    --cc=grant.likely@secretlab.ca \
    --cc=kernel-team@fb.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=pfeiner@google.com \
    --cc=siddhesh.poyarekar@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xemul@openvz.org \
    /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).