lkml.org 
[lkml]   [2017]   [Jan]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 36/83] ptrace: Dont allow accessing an undumpable mm
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Eric W. Biederman <ebiederm@xmission.com>

    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>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    arch/alpha/kernel/ptrace.c | 2 -
    arch/blackfin/kernel/ptrace.c | 4 +--
    arch/cris/arch-v32/kernel/ptrace.c | 2 -
    arch/ia64/kernel/ptrace.c | 2 -
    arch/mips/kernel/ptrace32.c | 4 +--
    arch/powerpc/kernel/ptrace32.c | 4 +--
    include/linux/mm.h | 2 +
    include/linux/ptrace.h | 3 ++
    kernel/ptrace.c | 42 +++++++++++++++++++++++++++++++------
    mm/memory.c | 2 -
    mm/nommu.c | 2 -
    11 files changed, 52 insertions(+), 17 deletions(-)

    --- a/arch/alpha/kernel/ptrace.c
    +++ b/arch/alpha/kernel/ptrace.c
    @@ -283,7 +283,7 @@ long arch_ptrace(struct task_struct *chi
    /* 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))
    --- a/arch/blackfin/kernel/ptrace.c
    +++ b/arch/blackfin/kernel/ptrace.c
    @@ -270,7 +270,7 @@ long arch_ptrace(struct task_struct *chi
    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 *chi
    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;
    --- 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 *chi
    /* 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;
    --- a/arch/ia64/kernel/ptrace.c
    +++ b/arch/ia64/kernel/ptrace.c
    @@ -1159,7 +1159,7 @@ arch_ptrace (struct task_struct *child,
    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;
    --- a/arch/mips/kernel/ptrace32.c
    +++ b/arch/mips/kernel/ptrace32.c
    @@ -69,7 +69,7 @@ long compat_arch_ptrace(struct task_stru
    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_stru
    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;
    --- a/arch/powerpc/kernel/ptrace32.c
    +++ b/arch/powerpc/kernel/ptrace32.c
    @@ -73,7 +73,7 @@ long compat_arch_ptrace(struct task_stru
    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_stru
    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;
    --- a/include/linux/mm.h
    +++ b/include/linux/mm.h
    @@ -1270,6 +1270,8 @@ extern int access_process_vm(struct task
    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,
    --- 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
    *
    --- 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 *
    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
    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_
    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_
    {
    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_st
    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_st

    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;
    --- 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;
    --- a/mm/nommu.c
    +++ b/mm/nommu.c
    @@ -1808,7 +1808,7 @@ void filemap_map_pages(struct fault_env
    }
    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;

    \
     
     \ /
      Last update: 2017-01-04 21:27    [W:4.190 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site