deal with races in /proc/*/{syscall,stack,personality}
authorAl Viro <viro@zeniv.linux.org.uk>
Wed, 23 Mar 2011 19:52:50 +0000 (15:52 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Wed, 23 Mar 2011 21:01:18 +0000 (17:01 -0400)
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@zeniv.linux.org.uk>
fs/proc/base.c

index bc15df390ec49c47be7ac4b0ae1030d806b1db90..7d5bb8b9a4ffe630b5a074116f9856ca48022322 100644 (file)
@@ -347,6 +347,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->signal->cred_guard_mutex);
+       if (err)
+               return err;
+       if (!ptrace_may_access(task, PTRACE_MODE_ATTACH)) {
+               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
@@ -356,6 +373,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);
@@ -366,15 +384,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
 
@@ -537,18 +560,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 */
 
@@ -2775,8 +2802,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;
 }
 
 /*
@@ -2795,7 +2826,7 @@ 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_IRUGO, proc_pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
        REG("sched",      S_IRUGO|S_IWUSR, proc_pid_sched_operations),
@@ -2805,7 +2836,7 @@ static const struct pid_entry tgid_base_stuff[] = {
 #endif
        REG("comm",      S_IRUGO|S_IWUSR, proc_pid_set_comm_operations),
 #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),
@@ -2833,7 +2864,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),
@@ -3135,14 +3166,14 @@ 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_IRUGO, proc_pid_limits),
 #ifdef CONFIG_SCHED_DEBUG
        REG("sched",     S_IRUGO|S_IWUSR, proc_pid_sched_operations),
 #endif
        REG("comm",      S_IRUGO|S_IWUSR, proc_pid_set_comm_operations),
 #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),
@@ -3169,7 +3200,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),