From patchwork Thu Jul 21 13:13:36 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andy Whitcroft X-Patchwork-Id: 106054 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id E8C7CB6F87 for ; Thu, 21 Jul 2011 23:14:52 +1000 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Qjt5M-0008Rj-B6; Thu, 21 Jul 2011 13:14:40 +0000 Received: from adelie.canonical.com ([91.189.90.139]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1Qjt4Y-00080x-6P for kernel-team@lists.ubuntu.com; Thu, 21 Jul 2011 13:13:50 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by adelie.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1Qjt4Y-00051F-1U; Thu, 21 Jul 2011 13:13:50 +0000 Received: from [85.210.154.17] (helo=localhost.localdomain) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1Qjt4X-0007zP-Qc; Thu, 21 Jul 2011 13:13:50 +0000 From: Andy Whitcroft To: kernel-team@lists.ubuntu.com Subject: [lucid, lucid/fsl-imx51 CVE 5/5] deal with races in /proc/*/{syscall, stack, personality} Date: Thu, 21 Jul 2011 14:13:36 +0100 Message-Id: <1311254026-29719-7-git-send-email-apw@canonical.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1311254026-29719-1-git-send-email-apw@canonical.com> References: <1311254026-29719-1-git-send-email-apw@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: kernel-team-bounces@lists.ubuntu.com Errors-To: kernel-team-bounces@lists.ubuntu.com From: Al Viro All of those are rw-r--r-- and all are broken for suid - if you open a file before the target does suid-root exec, you'll be still able to access it. For personality it's not a big deal, but for syscall and stack it's a real problem. Fix: check that task is tracable for you at the time of read(). Signed-off-by: Al Viro (backported from commit a9712bc12c40c172e393f85a9b2ba8db4bf59509) CVE-2011-1020 BugLink: http://bugs.launchpad.net/bugs/813026 Signed-off-by: Andy Whitcroft --- fs/proc/base.c | 69 ++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 50 insertions(+), 19 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index e901438..80d3a59 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -330,6 +330,23 @@ static int proc_pid_wchan(struct task_struct *task, char *buffer) } #endif /* CONFIG_KALLSYMS */ +static int lock_trace(struct task_struct *task) +{ + int err = mutex_lock_killable(&task->cred_guard_mutex); + if (err) + return err; + if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) { + mutex_unlock(&task->cred_guard_mutex); + return -EPERM; + } + return 0; +} + +static void unlock_trace(struct task_struct *task) +{ + mutex_unlock(&task->cred_guard_mutex); +} + #ifdef CONFIG_STACKTRACE #define MAX_STACK_TRACE_DEPTH 64 @@ -339,6 +356,7 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns, { struct stack_trace trace; unsigned long *entries; + int err; int i; entries = kmalloc(MAX_STACK_TRACE_DEPTH * sizeof(*entries), GFP_KERNEL); @@ -349,15 +367,20 @@ static int proc_pid_stack(struct seq_file *m, struct pid_namespace *ns, trace.max_entries = MAX_STACK_TRACE_DEPTH; trace.entries = entries; trace.skip = 0; - save_stack_trace_tsk(task, &trace); - for (i = 0; i < trace.nr_entries; i++) { - seq_printf(m, "[<%p>] %pS\n", - (void *)entries[i], (void *)entries[i]); + err = lock_trace(task); + if (!err) { + save_stack_trace_tsk(task, &trace); + + for (i = 0; i < trace.nr_entries; i++) { + seq_printf(m, "[<%p>] %pS\n", + (void *)entries[i], (void *)entries[i]); + } + unlock_trace(task); } kfree(entries); - return 0; + return err; } #endif @@ -529,18 +552,22 @@ static int proc_pid_syscall(struct task_struct *task, char *buffer) { long nr; unsigned long args[6], sp, pc; + int res = lock_trace(task); + if (res) + return res; if (task_current_syscall(task, &nr, args, 6, &sp, &pc)) - return sprintf(buffer, "running\n"); - - if (nr < 0) - return sprintf(buffer, "%ld 0x%lx 0x%lx\n", nr, sp, pc); - - return sprintf(buffer, + res = sprintf(buffer, "running\n"); + else if (nr < 0) + res = sprintf(buffer, "%ld 0x%lx 0x%lx\n", nr, sp, pc); + else + res = sprintf(buffer, "%ld 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx 0x%lx\n", nr, args[0], args[1], args[2], args[3], args[4], args[5], sp, pc); + unlock_trace(task); + return res; } #endif /* CONFIG_HAVE_ARCH_TRACEHOOK */ @@ -2494,8 +2521,12 @@ static int proc_tgid_io_accounting(struct task_struct *task, char *buffer) static int proc_pid_personality(struct seq_file *m, struct pid_namespace *ns, struct pid *pid, struct task_struct *task) { - seq_printf(m, "%08x\n", task->personality); - return 0; + int err = lock_trace(task); + if (!err) { + seq_printf(m, "%08x\n", task->personality); + unlock_trace(task); + } + return err; } /* @@ -2514,13 +2545,13 @@ static const struct pid_entry tgid_base_stuff[] = { REG("environ", S_IRUSR, proc_environ_operations), INF("auxv", S_IRUSR, proc_pid_auxv), ONE("status", S_IRUGO, proc_pid_status), - ONE("personality", S_IRUSR, proc_pid_personality), + ONE("personality", S_IRUGO, proc_pid_personality), INF("limits", S_IRUSR, proc_pid_limits), #ifdef CONFIG_SCHED_DEBUG REG("sched", S_IRUGO|S_IWUSR, proc_pid_sched_operations), #endif #ifdef CONFIG_HAVE_ARCH_TRACEHOOK - INF("syscall", S_IRUSR, proc_pid_syscall), + INF("syscall", S_IRUGO, proc_pid_syscall), #endif INF("cmdline", S_IRUGO, proc_pid_cmdline), ONE("stat", S_IRUGO, proc_tgid_stat), @@ -2548,7 +2579,7 @@ static const struct pid_entry tgid_base_stuff[] = { INF("wchan", S_IRUGO, proc_pid_wchan), #endif #ifdef CONFIG_STACKTRACE - ONE("stack", S_IRUSR, proc_pid_stack), + ONE("stack", S_IRUGO, proc_pid_stack), #endif #ifdef CONFIG_SCHEDSTATS INF("schedstat", S_IRUGO, proc_pid_schedstat), @@ -2853,13 +2884,13 @@ static const struct pid_entry tid_base_stuff[] = { REG("environ", S_IRUSR, proc_environ_operations), INF("auxv", S_IRUSR, proc_pid_auxv), ONE("status", S_IRUGO, proc_pid_status), - ONE("personality", S_IRUSR, proc_pid_personality), + ONE("personality", S_IRUGO, proc_pid_personality), INF("limits", S_IRUSR, proc_pid_limits), #ifdef CONFIG_SCHED_DEBUG REG("sched", S_IRUGO|S_IWUSR, proc_pid_sched_operations), #endif #ifdef CONFIG_HAVE_ARCH_TRACEHOOK - INF("syscall", S_IRUSR, proc_pid_syscall), + INF("syscall", S_IRUGO, proc_pid_syscall), #endif INF("cmdline", S_IRUGO, proc_pid_cmdline), ONE("stat", S_IRUGO, proc_tid_stat), @@ -2886,7 +2917,7 @@ static const struct pid_entry tid_base_stuff[] = { INF("wchan", S_IRUGO, proc_pid_wchan), #endif #ifdef CONFIG_STACKTRACE - ONE("stack", S_IRUSR, proc_pid_stack), + ONE("stack", S_IRUGO, proc_pid_stack), #endif #ifdef CONFIG_SCHEDSTATS INF("schedstat", S_IRUGO, proc_pid_schedstat),