ptrace: Don't allow accessing an undumpable mm

commit 84d77d3f06e7e8dea057d10e8ec77ad71f721be3 upstream.

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
Reported-by: Andy Lutomirski <luto@amacapital.net>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
[bwh: Backported to 3.16:
 - Pass around only a write flag, not gup_flags
 - Adjust context]
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>

diff --git a/arch/alpha/kernel/ptrace.c b/arch/alpha/kernel/ptrace.c
index 86d835157b546f08f7eff0cc58a5f3dccbc5b359..292e6cae1d101e6a5b1e12afd7d32a9b33c5f18c 100644 (file)
--- a/arch/alpha/kernel/ptrace.c
+++ b/arch/alpha/kernel/ptrace.c
@@ -281,7 +281,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), 0);
+               copied = ptrace_access_vm(child, addr, &tmp, sizeof(tmp), 0);
                ret = -EIO;
                if (copied != sizeof(tmp))
                        break;

diff --git a/arch/blackfin/kernel/ptrace.c b/arch/blackfin/kernel/ptrace.c
index 8b8fe671b1a6dbf347dd4d31a7f272ef87154bcb..7d8ece6a93fb5866adad88d0b84871d038e56b2b 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, 0);
                                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, 1);
                                break;
                        case BFIN_MEM_ACCESS_DMA:

diff --git a/arch/cris/arch-v32/kernel/ptrace.c b/arch/cris/arch-v32/kernel/ptrace.c
index f085229cf870bc306b95df2dee4eadd9ebdd4b8b..04251c6cb5f9dd0eb16099f87402f83ef64ccbaf 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), 0);
+                               copied = ptrace_access_vm(child, addr, &tmp, sizeof(tmp), 0);
 
                                if (copied != sizeof(tmp))
                                        break;

diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index b7a5fffe0924e03c860f50afcebc5bc0fc4ea7a3..c218249c4792bf63128d40f5d2533313aa695159 100644 (file)
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -1156,7 +1156,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), 0)
+               if (ptrace_access_vm(child, addr, &data, sizeof(data), 0)
                    != sizeof(data))
                        return -EIO;
                /* ensure return value is not mistaken for error code */

diff --git a/arch/mips/kernel/ptrace32.c b/arch/mips/kernel/ptrace32.c
index a83fb730b387a6174d4f46173fc4f2252cc79c02..fdd26c104c6caf73e0b8736cc9473e68fcf5f1cd 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), 0);
                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), 1) == sizeof(data))
                        break;
                ret = -EIO;

diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
index f52b7db327c80a3b603fb2a7179a2f97d6a054be..2e4f01dc9d6436179869aaef608a7a2b0c64b8c7 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), 0);
                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), 1) == sizeof(tmp))
                        break;
                ret = -EIO;

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 35f8b59b37a78e142085a66cdfff5c0879f5e879..65a4dc2da9de41cd5443e4e6ae64ba539783818f 100644 (file)
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1209,6 +1209,8 @@ static inline int fixup_user_fault(struct task_struct *tsk,
 extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write);
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
                void *buf, int len, int write);
+extern int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
+               unsigned long addr, void *buf, int len, int write);
 
 long __get_user_pages(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 b9b35c4d687b82ede057d7ed7309e536fa6cac51..8e81197a28dd666df0342db63bb727bf01a68430 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, int write);
+
 /*
  * Ptrace flags
  *

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index b48bb2fef8c936e499944c900234066c6047e973..630bd8abaab1a983c1384417adfe07477507d0a7 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, int write)
+{
+       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, write);
+       mmput(mm);
+
+       return ret;
+}
+
 
 static int ptrace_trapping_sleep_fn(void *flags)
 {
@@ -558,7 +587,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, 0);
+               retval = ptrace_access_vm(tsk, src, buf, this_len, 0);
+
                if (!retval) {
                        if (copied)
                                break;
@@ -585,7 +615,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, 1);
+               retval = ptrace_access_vm(tsk, dst, buf, this_len, 1);
                if (!retval) {
                        if (copied)
                                break;
@@ -1130,7 +1160,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), 0);
+       copied = ptrace_access_vm(tsk, addr, &tmp, sizeof(tmp), 0);
        if (copied != sizeof(tmp))
                return -EIO;
        return put_user(tmp, (unsigned long __user *)data);
@@ -1141,7 +1171,7 @@ int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
 {
        int copied;
 
-       copied = access_process_vm(tsk, addr, &data, sizeof(data), 1);
+       copied = ptrace_access_vm(tsk, addr, &data, sizeof(data), 1);
        return (copied == sizeof(data)) ? 0 : -EIO;
 }
 
@@ -1159,7 +1189,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), 0);
+               ret = ptrace_access_vm(child, addr, &word, sizeof(word), 0);
                if (ret != sizeof(word))
                        ret = -EIO;
                else
@@ -1168,7 +1198,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), 1);
+               ret = ptrace_access_vm(child, addr, &data, sizeof(data), 1);
                ret = (ret != sizeof(data) ? -EIO : 0);
                break;
 

diff --git a/mm/memory.c b/mm/memory.c
index e1bde862d6d3f3055ab836f6a1a965139b41dfeb..4ab5b0c55f4ad2b86e987f26215c9398ed44aa6c 100644 (file)
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3560,7 +3560,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, int write)
 {
        struct vm_area_struct *vma;

diff --git a/mm/nommu.c b/mm/nommu.c
index 53a59f41e2e5504202f75b2d5c6a46aeeef51163..7f2c901c555e435d8ded3f37ab96d70a25586d3b 100644 (file)
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -2007,7 +2007,7 @@ int generic_file_remap_pages(struct vm_area_struct *vma, unsigned long addr,
 }
 EXPORT_SYMBOL(generic_file_remap_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, int write)
 {
        struct vm_area_struct *vma;