APPLIED: [lucid, lucid/fsl-imx51 CVE 5/5] deal with races in /proc/*/{syscall, stack, personality}
Tim Gardner
tim.gardner at canonical.com
Thu Jul 21 18:13:58 UTC 2011
On 07/21/2011 06:13 AM, Andy Whitcroft wrote:
> From: Al Viro<viro at zeniv.linux.org.uk>
>
> 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<viro at zeniv.linux.org.uk>
>
> (backported from commit a9712bc12c40c172e393f85a9b2ba8db4bf59509)
> CVE-2011-1020
> BugLink: http://bugs.launchpad.net/bugs/813026
> Signed-off-by: Andy Whitcroft<apw at canonical.com>
> ---
> 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),
--
Tim Gardner tim.gardner at canonical.com
More information about the kernel-team
mailing list