ptrace: Don't allow accessing an undumpable mm

It is the reasonable expectation that if an executable file is not
readable there will be no way for a user without special privileges to
read the file.  This is enforced in ptrace_attach but if ptrace
is already attached before exec there is no enforcement for read-only
executables.

As the only way to read such an mm is through access_process_vm
spin a variant called ptrace_access_vm that will fail if the
target process is not being ptraced by the current process, or
the current process did not have sufficient privileges when ptracing
began to read the target processes mm.

In the ptrace implementations replace access_process_vm by
ptrace_access_vm.  There remain several ptrace sites that still use
access_process_vm as they are reading the target executables
instructions (for kernel consumption) or register stacks.  As such it
does not appear necessary to add a permission check to those calls.

This bug has always existed in Linux.

Fixes: v1.0
Cc: stable@vger.kernel.org
Reported-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index 940dfb4065910822d42f3d11c02ba2305f25b02a..04abdec7f49605ecee1b460162440b6cfdb3f554 100644 (file)
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -283,7 +283,7 @@ long arch_ptrace(struct task_struct *child, long request,
        /* When I and D space are separate, these will need to be fixed.  */
        case PTRACE_PEEKTEXT: /* read word at location addr. */
        case PTRACE_PEEKDATA:
-               copied = access_process_vm(child, addr, &tmp, sizeof(tmp),
+               copied = ptrace_access_vm(child, addr, &tmp, sizeof(tmp),
                                FOLL_FORCE);
                ret = -EIO;
                if (copied != sizeof(tmp))

diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index 8d79286ee4e878044b02c963f5fb3c6deaaafb19..360d996451633c8281256048305f88c245ae3819 100644 (file)
--- a/arch/blackfin/kernel/ptrace.c
+++ b/arch/blackfin/kernel/ptrace.c
@@ -270,7 +270,7 @@ long arch_ptrace(struct task_struct *child, long request,
                        switch (bfin_mem_access_type(addr, to_copy)) {
                        case BFIN_MEM_ACCESS_CORE:
                        case BFIN_MEM_ACCESS_CORE_ONLY:
-                               copied = access_process_vm(child, addr, &tmp,
+                               copied = ptrace_access_vm(child, addr, &tmp,
                                                           to_copy, FOLL_FORCE);
                                if (copied)
                                        break;
@@ -323,7 +323,7 @@ long arch_ptrace(struct task_struct *child, long request,
                        switch (bfin_mem_access_type(addr, to_copy)) {
                        case BFIN_MEM_ACCESS_CORE:
                        case BFIN_MEM_ACCESS_CORE_ONLY:
-                               copied = access_process_vm(child, addr, &data,
+                               copied = ptrace_access_vm(child, addr, &data,
                                                           to_copy,
                                                           FOLL_FORCE | FOLL_WRITE);
                                break;

diff --git a/arch/cris/arch-v32/kernel/ptrace.c b/arch/cris/arch-v32/kernel/ptrace.c
index f0df654ac6fc5ca53ffd6d6951817e92f492e0be..fe1f9cf7b391e54c9b973b07fe00d29ab6099ec5 100644 (file)
--- a/arch/cris/arch-v32/kernel/ptrace.c
+++ b/arch/cris/arch-v32/kernel/ptrace.c
@@ -147,7 +147,7 @@ long arch_ptrace(struct task_struct *child, long request,
                                /* The trampoline page is globally mapped, no page table to traverse.*/
                                tmp = *(unsigned long*)addr;
                        } else {
-                               copied = access_process_vm(child, addr, &tmp, sizeof(tmp), FOLL_FORCE);
+                               copied = ptrace_access_vm(child, addr, &tmp, sizeof(tmp), FOLL_FORCE);
 
                                if (copied != sizeof(tmp))
                                        break;

diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index 31aa8c0f68e14a284e0f2b088afe4c29de7daee5..36f660da81242f6cb13be8ded36f4d80bad4759d 100644 (file)
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1159,7 +1159,7 @@ arch_ptrace (struct task_struct *child, long request,
        case PTRACE_PEEKTEXT:
        case PTRACE_PEEKDATA:
                /* read word at location addr */
-               if (access_process_vm(child, addr, &data, sizeof(data),
+               if (ptrace_access_vm(child, addr, &data, sizeof(data),
                                FOLL_FORCE)
                    != sizeof(data))
                        return -EIO;

diff --git a/arch/mips/kernel/ptrace32.c b/arch/mips/kernel/ptrace32.c
index 7e71a4e0281ba9cc3c2190dd9a06d958754155fb..5fcbdcd7abd0bcecff728726812aca54519608c7 100644 (file)
--- a/arch/mips/kernel/ptrace32.c
+++ b/arch/mips/kernel/ptrace32.c
@@ -69,7 +69,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                if (get_user(addrOthers, (u32 __user * __user *) (unsigned long) addr) != 0)
                        break;
 
-               copied = access_process_vm(child, (u64)addrOthers, &tmp,
+               copied = ptrace_access_vm(child, (u64)addrOthers, &tmp,
                                sizeof(tmp), FOLL_FORCE);
                if (copied != sizeof(tmp))
                        break;
@@ -178,7 +178,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                if (get_user(addrOthers, (u32 __user * __user *) (unsigned long) addr) != 0)
                        break;
                ret = 0;
-               if (access_process_vm(child, (u64)addrOthers, &data,
+               if (ptrace_access_vm(child, (u64)addrOthers, &data,
                                        sizeof(data),
                                        FOLL_FORCE | FOLL_WRITE) == sizeof(data))
                        break;

diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
index 010b7b310237e4be38ef1d7bcc15a76050fc4469..1e887f3a61a675a0dd9a7ff5aa29c2b205c809ec 100644 (file)
--- a/arch/powerpc/kernel/ptrace32.c
+++ b/arch/powerpc/kernel/ptrace32.c
@@ -73,7 +73,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                if (get_user(addrOthers, (u32 __user * __user *)addr) != 0)
                        break;
 
-               copied = access_process_vm(child, (u64)addrOthers, &tmp,
+               copied = ptrace_access_vm(child, (u64)addrOthers, &tmp,
                                sizeof(tmp), FOLL_FORCE);
                if (copied != sizeof(tmp))
                        break;
@@ -178,7 +178,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                if (get_user(addrOthers, (u32 __user * __user *)addr) != 0)
                        break;
                ret = 0;
-               if (access_process_vm(child, (u64)addrOthers, &tmp,
+               if (ptrace_access_vm(child, (u64)addrOthers, &tmp,
                                        sizeof(tmp),
                                        FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
                        break;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d73aeafc5f5bafa551eacc284f86bad50d0..0b5b2e4df14e909ba62c33d3704b9578ddbce88e 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1270,6 +1270,8 @@ extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void *
                unsigned int gup_flags);
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
                void *buf, int len, unsigned int gup_flags);
+extern int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
+               unsigned long addr, void *buf, int len, unsigned int gup_flags);
 
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
                            unsigned long start, unsigned long nr_pages,

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index e13bfdf7f314784e326b245e1f1ab5465da9e7dc..e0e539321ab95c38adb2ca9f28b358e04253c999 100644 (file)
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -8,6 +8,9 @@
 #include <linux/pid_namespace.h>       /* For task_active_pid_ns.  */
 #include <uapi/linux/ptrace.h>
 
+extern int ptrace_access_vm(struct task_struct *tsk, unsigned long addr,
+                           void *buf, int len, unsigned int gup_flags);
+
 /*
  * Ptrace flags
  *

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index e82c15cadd6df68dfbe7c8866485db59c3f8689d..49ba7c1ade9d074b814105fb648f33b0ce95f061 100644 (file)
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -27,6 +27,35 @@
 #include <linux/cn_proc.h>
 #include <linux/compat.h>
 
+/*
+ * Access another process' address space via ptrace.
+ * Source/target buffer must be kernel space,
+ * Do not walk the page table directly, use get_user_pages
+ */
+int ptrace_access_vm(struct task_struct *tsk, unsigned long addr,
+                    void *buf, int len, unsigned int gup_flags)
+{
+       struct mm_struct *mm;
+       int ret;
+
+       mm = get_task_mm(tsk);
+       if (!mm)
+               return 0;
+
+       if (!tsk->ptrace ||
+           (current != tsk->parent) ||
+           ((get_dumpable(mm) != SUID_DUMP_USER) &&
+            !ptracer_capable(tsk, mm->user_ns))) {
+               mmput(mm);
+               return 0;
+       }
+
+       ret = __access_remote_vm(tsk, mm, addr, buf, len, gup_flags);
+       mmput(mm);
+
+       return ret;
+}
+
 
 /*
  * ptrace a task: make the debugger its new parent and
@@ -535,7 +564,8 @@ int ptrace_readdata(struct task_struct *tsk, unsigned long src, char __user *dst
                int this_len, retval;
 
                this_len = (len > sizeof(buf)) ? sizeof(buf) : len;
-               retval = access_process_vm(tsk, src, buf, this_len, FOLL_FORCE);
+               retval = ptrace_access_vm(tsk, src, buf, this_len, FOLL_FORCE);
+
                if (!retval) {
                        if (copied)
                                break;
@@ -562,7 +592,7 @@ int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long ds
                this_len = (len > sizeof(buf)) ? sizeof(buf) : len;
                if (copy_from_user(buf, src, this_len))
                        return -EFAULT;
-               retval = access_process_vm(tsk, dst, buf, this_len,
+               retval = ptrace_access_vm(tsk, dst, buf, this_len,
                                FOLL_FORCE | FOLL_WRITE);
                if (!retval) {
                        if (copied)
@@ -1126,7 +1156,7 @@ int generic_ptrace_peekdata(struct task_struct *tsk, unsigned long addr,
        unsigned long tmp;
        int copied;
 
-       copied = access_process_vm(tsk, addr, &tmp, sizeof(tmp), FOLL_FORCE);
+       copied = ptrace_access_vm(tsk, addr, &tmp, sizeof(tmp), FOLL_FORCE);
        if (copied != sizeof(tmp))
                return -EIO;
        return put_user(tmp, (unsigned long __user *)data);
@@ -1137,7 +1167,7 @@ int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
 {
        int copied;
 
-       copied = access_process_vm(tsk, addr, &data, sizeof(data),
+       copied = ptrace_access_vm(tsk, addr, &data, sizeof(data),
                        FOLL_FORCE | FOLL_WRITE);
        return (copied == sizeof(data)) ? 0 : -EIO;
 }
@@ -1155,7 +1185,7 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
        switch (request) {
        case PTRACE_PEEKTEXT:
        case PTRACE_PEEKDATA:
-               ret = access_process_vm(child, addr, &word, sizeof(word),
+               ret = ptrace_access_vm(child, addr, &word, sizeof(word),
                                FOLL_FORCE);
                if (ret != sizeof(word))
                        ret = -EIO;
@@ -1165,7 +1195,7 @@ int compat_ptrace_request(struct task_struct *child, compat_long_t request,
 
        case PTRACE_POKETEXT:
        case PTRACE_POKEDATA:
-               ret = access_process_vm(child, addr, &data, sizeof(data),
+               ret = ptrace_access_vm(child, addr, &data, sizeof(data),
                                FOLL_FORCE | FOLL_WRITE);
                ret = (ret != sizeof(data) ? -EIO : 0);
                break;

diff --git a/mm/memory.c b/mm/memory.c
index e18c57bdc75c4c96e3ef79546afe11fab5a3c07a..cbb1e5e5f79159159d1f90fe607a9e512bdcaf53 100644 (file)
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3868,7 +3868,7 @@ EXPORT_SYMBOL_GPL(generic_access_phys);
  * Access another process' address space as given in mm.  If non-NULL, use the
  * given task for page fault accounting.
  */
-static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
+int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
                unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
        struct vm_area_struct *vma;

diff --git a/mm/nommu.c b/mm/nommu.c
index 8b8faaf2a9e95cfc607ff1a5a37c83eadfda59fe..44265e00b701ebe5bc3327c8575f5181aa6a6f99 100644 (file)
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -1808,7 +1808,7 @@ void filemap_map_pages(struct fault_env *fe,
 }
 EXPORT_SYMBOL(filemap_map_pages);
 
-static int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
+int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
                unsigned long addr, void *buf, int len, unsigned int gup_flags)
 {
        struct vm_area_struct *vma;