All of lore.kernel.org
 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: 80+ 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 21:00                 ` Cyrill Gorcunov
2015-01-26 23:43                 ` Andrew Morton
2015-01-27  0:15                   ` Kees Cook
2015-01-27  0:15                     ` Kees Cook
2015-01-27  7:37                     ` Cyrill Gorcunov
2015-01-27  7:37                       ` Cyrill Gorcunov
2015-01-27 19:53                       ` Kees Cook
2015-01-27 19:53                         ` Kees Cook
2015-01-27 21:35                         ` Cyrill Gorcunov
2015-01-27 21:35                           ` Cyrill Gorcunov
2015-01-27 21:46                         ` Pavel Emelyanov
2015-01-27 21:46                           ` Pavel Emelyanov
2015-01-27  0:19                   ` Kirill A. Shutemov
2015-01-27  0:19                     ` Kirill A. Shutemov
2015-01-27  6:46                   ` Cyrill Gorcunov
2015-01-27  6:46                     ` Cyrill Gorcunov
2015-01-27  6:50                     ` Andrew Morton
2015-01-27  7:23                       ` Cyrill Gorcunov
2015-01-27  7:23                         ` Cyrill Gorcunov
2015-01-28  4:38                   ` Calvin Owens
2015-01-28  4:38                     ` Calvin Owens
2015-01-30  1:30                     ` Kees Cook
2015-01-30  1:30                       ` Kees Cook
2015-01-31  1:58                       ` Calvin Owens
2015-01-31  1:58                         ` Calvin Owens
2015-02-02 14:01                         ` Austin S Hemmelgarn
2015-02-04  3:53                           ` Calvin Owens
2015-02-04  3:53                             ` Calvin Owens
2015-02-02 20:16                         ` Andy Lutomirski
2015-02-04  3:28                           ` Calvin Owens
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.