Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752866AbbESDKS (ORCPT ); Mon, 18 May 2015 23:10:18 -0400 Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:13238 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751966AbbESDKO (ORCPT ); Mon, 18 May 2015 23:10:14 -0400 From: Calvin Owens To: Andrew Morton , Alexey Dobriyan , "Eric W. Biederman" , Al Viro , Miklos Szeredi , Zefan Li , Oleg Nesterov , Joe Perches , David Howells CC: Calvin Owens , , , Andy Lutomirski , Kees Cook , "Kirill A. Shutemov" Subject: [PATCH v5] procfs: Always expose /proc//map_files/ and make it readable Date: Mon, 18 May 2015 20:10:06 -0700 Message-ID: <1432005006-3428-1-git-send-email-calvinowens@fb.com> X-Mailer: git-send-email 2.1.4 In-Reply-To: <20150214204009.GA1763278@mail.thefacebook.com> References: <20150214204009.GA1763278@mail.thefacebook.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [192.168.52.13] X-Proofpoint-Spam-Reason: safe X-FB-Internal: Safe X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.14.151,1.0.33,0.0.0000 definitions=2015-05-19_01:2015-05-18,2015-05-19,1970-01-01 signatures=0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6675 Lines: 196 Currently, /proc//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//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//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//mem is allowed, it will not allow an attacker to do anything that was not already possible through that interface. Cc: Andrew Morton Cc: Andy Lutomirski Cc: Kees Cook Cc: Kirill A. Shutemov Reviewed-by: Cyrill Gorcunov Signed-off-by: Calvin Owens --- 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//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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/