From patchwork Fri Sep 23 20:40:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 9348841 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0DFEC607F2 for ; Fri, 23 Sep 2016 20:41:04 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1C7E2ADDA for ; Fri, 23 Sep 2016 20:41:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E698B2ADDC; Fri, 23 Sep 2016 20:41:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3F1AA2ADDA for ; Fri, 23 Sep 2016 20:41:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1034487AbcIWUlA (ORCPT ); Fri, 23 Sep 2016 16:41:00 -0400 Received: from thejh.net ([37.221.195.125]:48211 "EHLO thejh.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761440AbcIWUkw (ORCPT ); Fri, 23 Sep 2016 16:40:52 -0400 Received: from pc.thejh.net (pc.vpn [192.168.44.2]) by thejh.net (Postfix) with ESMTPSA id C2BCB1806C6; Fri, 23 Sep 2016 22:40:49 +0200 (CEST) From: Jann Horn To: Alexander Viro , Roland McGrath , Oleg Nesterov , John Johansen , James Morris , "Serge E. Hallyn" , Paul Moore , Stephen Smalley , Eric Paris , Casey Schaufler , Kees Cook , Andrew Morton , Janis Danisevskis , Seth Forshee , "Eric . Biederman" , Thomas Gleixner , Benjamin LaHaise , Ben Hutchings , Andy Lutomirski , Linus Torvalds Cc: linux-fsdevel@vger.kernel.org, linux-security-module@vger.kernel.org, security@kernel.org Subject: [PATCH v2 5/8] proc: lock properly in ptrace_may_access callers Date: Fri, 23 Sep 2016 22:40:35 +0200 Message-Id: <1474663238-22134-6-git-send-email-jann@thejh.net> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1474663238-22134-1-git-send-email-jann@thejh.net> References: <1474663238-22134-1-git-send-email-jann@thejh.net> Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP Use the new cred_guard_light to prevent information leaks through races in procfs. changed in v2: - also use mm_access() for proc_map_files_readdir() (0day test robot) Signed-off-by: Jann Horn --- fs/proc/array.c | 7 +++ fs/proc/base.c | 134 ++++++++++++++++++++++++++++++--------------------- fs/proc/namespaces.c | 14 ++++++ 3 files changed, 99 insertions(+), 56 deletions(-) diff --git a/fs/proc/array.c b/fs/proc/array.c index 3349742..c28f254 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -410,9 +410,15 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, unsigned long rsslim = 0; char tcomm[sizeof(task->comm)]; unsigned long flags; + int err; state = *get_task_state(task); vsize = eip = esp = 0; + + err = mutex_lock_killable(&task->signal->cred_guard_light); + if (err) + return err; + permitted = proc_ptrace_may_access_seq(task, PTRACE_MODE_READ_FSCREDS | PTRACE_MODE_NOAUDIT, m); mm = get_task_mm(task); @@ -568,6 +574,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns, seq_putc(m, '\n'); if (mm) mmput(mm); + mutex_unlock(&task->signal->cred_guard_light); return 0; } diff --git a/fs/proc/base.c b/fs/proc/base.c index d6c98ab..15845cf 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -135,6 +135,25 @@ struct pid_entry { NULL, &proc_single_file_operations, \ { .proc_show = show } ) +static int lock_trace(struct seq_file *m, struct task_struct *task, + unsigned int mode) +{ + int err = mutex_lock_killable(&task->signal->cred_guard_light); + + if (err) + return err; + if (!proc_ptrace_may_access_seq(task, mode | PTRACE_MODE_FSCREDS, m)) { + mutex_unlock(&task->signal->cred_guard_light); + return -EPERM; + } + return 0; +} + +static void unlock_trace(struct task_struct *task) +{ + mutex_unlock(&task->signal->cred_guard_light); +} + /* * Count the number of hardlinks for the pid_entry table, excluding the . * and .. links. @@ -428,36 +447,20 @@ static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns, unsigned long wchan; char symname[KSYM_NAME_LEN]; - wchan = get_wchan(task); - - if (wchan && - proc_ptrace_may_access_seq(task, PTRACE_MODE_READ_FSCREDS, m) && - !lookup_symbol_name(wchan, symname)) - seq_printf(m, "%s", symname); - else - seq_putc(m, '0'); + if (lock_trace(m, task, PTRACE_MODE_READ) == 0) { + wchan = get_wchan(task); + unlock_trace(task); + if (wchan && !lookup_symbol_name(wchan, symname)) { + seq_printf(m, "%s", symname); + return 0; + } + } + seq_putc(m, '0'); return 0; } #endif /* CONFIG_KALLSYMS */ -static int lock_trace(struct seq_file *m, struct task_struct *task) -{ - int err = mutex_lock_killable(&task->signal->cred_guard_mutex); - if (err) - return err; - if (!proc_ptrace_may_access_seq(task, PTRACE_MODE_ATTACH_FSCREDS, m)) { - mutex_unlock(&task->signal->cred_guard_mutex); - return -EPERM; - } - return 0; -} - -static void unlock_trace(struct task_struct *task) -{ - mutex_unlock(&task->signal->cred_guard_mutex); -} - #ifdef CONFIG_STACKTRACE #define MAX_STACK_TRACE_DEPTH 64 @@ -479,7 +482,7 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns, trace.entries = entries; trace.skip = 0; - err = lock_trace(m, task); + err = lock_trace(m, task, PTRACE_MODE_ATTACH); if (!err) { save_stack_trace_tsk(task, &trace); @@ -661,7 +664,7 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns, unsigned long args[6], sp, pc; int res; - res = lock_trace(m, task); + res = lock_trace(m, task, PTRACE_MODE_ATTACH); if (res) return res; @@ -685,23 +688,38 @@ static int proc_pid_syscall(struct seq_file *m, struct pid_namespace *ns, /* Here the fs part begins */ /************************************************************************/ -/* permission checks */ -static int proc_fd_access_allowed(struct inode *inode) +/* permission checks. + * If this returns 1, you'll have to unlock_trace(*taskp) afterwards. + */ +static int proc_fd_access_allowed_lock(struct inode *inode, + struct task_struct **taskp) { struct task_struct *task; int allowed = 0; - /* Allow access to a task's file descriptors if it is us or we - * may use ptrace attach to the process and find out that - * information. - */ + task = get_proc_task(inode); - if (task) { - allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS); + if (!task) + return 0; + if (mutex_lock_killable(&task->signal->cred_guard_light)) + goto out_put; + allowed = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS); + if (!allowed) + mutex_unlock(&task->signal->cred_guard_light); +out_put: + if (allowed) + *taskp = task; + else put_task_struct(task); - } + return allowed; } +static void proc_fd_access_allowed_unlock(struct task_struct *task) +{ + mutex_unlock(&task->signal->cred_guard_light); + put_task_struct(task); +} + int proc_setattr(struct dentry *dentry, struct iattr *attr) { int error; @@ -722,6 +740,7 @@ int proc_setattr(struct dentry *dentry, struct iattr *attr) /* * May current process learn task's sched/cmdline info (for hide_pid_min=1) * or euid/egid (for hide_pid_min=2)? + * NOTE: When you call this, you must hold cred_guard_mutex or cred_guard_light. */ static bool has_pid_permissions(struct pid_namespace *pid, struct task_struct *task, @@ -1600,15 +1619,17 @@ static const char *proc_pid_get_link(struct dentry *dentry, { struct path path; int error = -EACCES; + struct task_struct *task; if (!dentry) return ERR_PTR(-ECHILD); /* Are we allowed to snoop on the tasks file descriptors? */ - if (!proc_fd_access_allowed(inode)) + if (!proc_fd_access_allowed_lock(inode, &task)) goto out; error = PROC_I(inode)->op.proc_get_link(dentry, &path); + proc_fd_access_allowed_unlock(task); if (error) goto out; @@ -1647,12 +1668,14 @@ static int proc_pid_readlink(struct dentry * dentry, char __user * buffer, int b int error = -EACCES; struct inode *inode = d_inode(dentry); struct path path; + struct task_struct *task; /* Are we allowed to snoop on the tasks file descriptors? */ - if (!proc_fd_access_allowed(inode)) + if (!proc_fd_access_allowed_lock(inode, &task)) goto out; error = PROC_I(inode)->op.proc_get_link(dentry, &path); + proc_fd_access_allowed_unlock(task); if (error) goto out; @@ -2047,17 +2070,15 @@ static struct dentry *proc_map_files_lookup(struct inode *dir, if (!task) goto out; - result = -EACCES; - if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) + mm = mm_access(task, PTRACE_MODE_READ_FSCREDS); + if (IS_ERR(mm)) + result = PTR_ERR(mm); + if (IS_ERR_OR_NULL(mm)) goto out_put_task; result = -ENOENT; if (dname_to_vma_addr(dentry, &vm_start, &vm_end)) - goto out_put_task; - - mm = get_task_mm(task); - if (!mm) - goto out_put_task; + goto out_put_mm; down_read(&mm->mmap_sem); vma = find_exact_vma(mm, vm_start, vm_end); @@ -2070,6 +2091,7 @@ static struct dentry *proc_map_files_lookup(struct inode *dir, out_no_vma: up_read(&mm->mmap_sem); +out_put_mm: mmput(mm); out_put_task: put_task_struct(task); @@ -2100,17 +2122,17 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx) if (!task) goto out; - ret = -EACCES; - if (!ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) + ret = 0; + + mm = mm_access(task, PTRACE_MODE_READ_FSCREDS); + if (IS_ERR(mm)) + ret = PTR_ERR(mm); + if (IS_ERR_OR_NULL(mm)) goto out_put_task; - ret = 0; if (!dir_emit_dots(file, ctx)) - goto out_put_task; + goto out_mmput; - mm = get_task_mm(task); - if (!mm) - goto out_put_task; down_read(&mm->mmap_sem); nr_files = 0; @@ -2139,8 +2161,7 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx) if (fa) flex_array_free(fa); up_read(&mm->mmap_sem); - mmput(mm); - goto out_put_task; + goto out_mmput; } for (i = 0, vma = mm->mmap, pos = 2; vma; vma = vma->vm_next) { @@ -2171,8 +2192,9 @@ proc_map_files_readdir(struct file *file, struct dir_context *ctx) } if (fa) flex_array_free(fa); - mmput(mm); +out_mmput: + mmput(mm); out_put_task: put_task_struct(task); out: @@ -2839,7 +2861,7 @@ static const struct file_operations proc_setgroups_operations = { static int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns, struct pid *pid, struct task_struct *task) { - int err = lock_trace(m, task); + int err = lock_trace(m, task, PTRACE_MODE_ATTACH); if (!err) { seq_printf(m, "%08x\n", task->personality); unlock_trace(task); diff --git a/fs/proc/namespaces.c b/fs/proc/namespaces.c index 51b8b0a..e1246e8 100644 --- a/fs/proc/namespaces.c +++ b/fs/proc/namespaces.c @@ -49,11 +49,18 @@ static const char *proc_ns_get_link(struct dentry *dentry, if (!task) return error; + error = ERR_PTR(mutex_lock_killable(&task->signal->cred_guard_light)); + if (error) + goto out_put_task; + + error = ERR_PTR(-EACCES); if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) { error = ns_get_path(&ns_path, task, ns_ops); if (!error) nd_jump_link(&ns_path); } + mutex_unlock(&task->signal->cred_guard_light); +out_put_task: put_task_struct(task); return error; } @@ -70,11 +77,18 @@ static int proc_ns_readlink(struct dentry *dentry, char __user *buffer, int bufl if (!task) return res; + res = mutex_lock_killable(&task->signal->cred_guard_light); + if (res) + goto out_put_task; + + res = -EACCES; if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS)) { res = ns_get_name(name, sizeof(name), task, ns_ops); if (res >= 0) res = readlink_copy(buffer, buflen, name); } + mutex_unlock(&task->signal->cred_guard_light); +out_put_task: put_task_struct(task); return res; }