linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Calvin Owens <calvinowens@fb.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Miklos Szeredi <miklos@szeredi.hu>, Zefan Li <lizefan@huawei.com>,
	Oleg Nesterov <oleg@redhat.com>, Joe Perches <joe@perches.com>,
	David Howells <dhowells@redhat.com>
Cc: Calvin Owens <calvinowens@fb.com>, <linux-kernel@vger.kernel.org>,
	<kernel-team@fb.com>, Andy Lutomirski <luto@amacapital.net>,
	Kees Cook <keescook@chromium.org>,
	"Kirill A. Shutemov" <kirill@shutemov.name>
Subject: [PATCH v5] procfs: Always expose /proc/<pid>/map_files/ and make it readable
Date: Mon, 18 May 2015 20:10:06 -0700	[thread overview]
Message-ID: <1432005006-3428-1-git-send-email-calvinowens@fb.com> (raw)
In-Reply-To: <20150214204009.GA1763278@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
very 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,
following the links 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.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Kees Cook <keescook@chromium.org>
Cc: Kirill A. Shutemov <kirill@shutemov.name>
Reviewed-by: Cyrill Gorcunov <gorcunov@openvz.org>
Signed-off-by: Calvin Owens <calvinowens@fb.com>
---
Changes in v5:	s/dentry->d_inode/d_inode(dentry)/g

Changes in v4:	Return -ESRCH from follow_link() when get_proc_task()
		returns NULL.

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 | 61 +++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 093ca14..22d95a7 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -1641,8 +1641,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.
@@ -1669,17 +1667,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 = d_inode(dentry);
 	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;
 
@@ -1762,6 +1755,41 @@ 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 = d_inode(dentry);
+	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);
+	} else {
+		return ERR_PTR(-ESRCH);
+	}
+
+	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)
@@ -1777,7 +1805,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;
 
@@ -1801,17 +1829,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;
@@ -1858,17 +1882,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;
@@ -2050,7 +2070,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)
@@ -2549,9 +2568,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-05-19  3:10 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             ` [RFC][PATCH v3] " Calvin Owens
2015-02-12  7:45               ` 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                 ` Calvin Owens [this message]
2015-05-19  3:29                   ` [PATCH v5] " 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=1432005006-3428-1-git-send-email-calvinowens@fb.com \
    --to=calvinowens@fb.com \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=dhowells@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=joe@perches.com \
    --cc=keescook@chromium.org \
    --cc=kernel-team@fb.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=luto@amacapital.net \
    --cc=miklos@szeredi.hu \
    --cc=oleg@redhat.com \
    --cc=viro@zeniv.linux.org.uk \
    /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).