procfs: mem permission cleanup
authorRoland McGrath <roland@redhat.com>
Tue, 29 Apr 2008 08:01:38 +0000 (01:01 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Tue, 29 Apr 2008 15:06:17 +0000 (08:06 -0700)
This cleans up the permission checks done for /proc/PID/mem i/o calls.  It
puts all the logic in a new function, check_mem_permission().

The old code repeated the (!MAY_PTRACE(task) || !ptrace_may_attach(task))
magical expression multiple times.  The new function does all that work in one
place, with clear comments.

The old code called security_ptrace() twice on successful checks, once in
MAY_PTRACE() and once in __ptrace_may_attach().  Now it's only called once,
and only if all other checks have succeeded.

Signed-off-by: Roland McGrath <roland@redhat.com>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
fs/proc/base.c

index b48ddb1..fcf02f2 100644 (file)
@@ -195,12 +195,32 @@ static int proc_root_link(struct inode *inode, struct path *path)
        return result;
 }
 
-#define MAY_PTRACE(task) \
-       (task == current || \
-       (task->parent == current && \
-       (task->ptrace & PT_PTRACED) && \
-        (task_is_stopped_or_traced(task)) && \
-        security_ptrace(current,task) == 0))
+/*
+ * Return zero if current may access user memory in @task, -error if not.
+ */
+static int check_mem_permission(struct task_struct *task)
+{
+       /*
+        * A task can always look at itself, in case it chooses
+        * to use system calls instead of load instructions.
+        */
+       if (task == current)
+               return 0;
+
+       /*
+        * If current is actively ptrace'ing, and would also be
+        * permitted to freshly attach with ptrace now, permit it.
+        */
+       if (task->parent == current && (task->ptrace & PT_PTRACED) &&
+           task_is_stopped_or_traced(task) &&
+           ptrace_may_attach(task))
+               return 0;
+
+       /*
+        * Noone else is allowed.
+        */
+       return -EPERM;
+}
 
 struct mm_struct *mm_for_maps(struct task_struct *task)
 {
@@ -722,7 +742,7 @@ static ssize_t mem_read(struct file * file, char __user * buf,
        if (!task)
                goto out_no_task;
 
-       if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
+       if (check_mem_permission(task))
                goto out;
 
        ret = -ENOMEM;
@@ -748,7 +768,7 @@ static ssize_t mem_read(struct file * file, char __user * buf,
 
                this_len = (count > PAGE_SIZE) ? PAGE_SIZE : count;
                retval = access_process_vm(task, src, page, this_len, 0);
-               if (!retval || !MAY_PTRACE(task) || !ptrace_may_attach(task)) {
+               if (!retval || check_mem_permission(task)) {
                        if (!ret)
                                ret = -EIO;
                        break;
@@ -792,7 +812,7 @@ static ssize_t mem_write(struct file * file, const char __user *buf,
        if (!task)
                goto out_no_task;
 
-       if (!MAY_PTRACE(task) || !ptrace_may_attach(task))
+       if (check_mem_permission(task))
                goto out;
 
        copied = -ENOMEM;