mm: replace access_process_vm() write parameter with gup_flags
authorLorenzo Stoakes <lstoakes@gmail.com>
Thu, 13 Oct 2016 00:20:20 +0000 (01:20 +0100)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 19 Oct 2016 15:31:25 +0000 (08:31 -0700)
This removes the 'write' argument from access_process_vm() and replaces
it with 'gup_flags' as use of this function previously silently implied
FOLL_FORCE, whereas after this patch callers explicitly pass this flag.

We make this explicit as use of FOLL_FORCE can result in surprising
behaviour (and hence bugs) within the mm subsystem.

Signed-off-by: Lorenzo Stoakes <lstoakes@gmail.com>
Acked-by: Jesper Nilsson <jesper.nilsson@axis.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
17 files changed:
arch/alpha/kernel/ptrace.c
arch/blackfin/kernel/ptrace.c
arch/cris/arch-v32/kernel/ptrace.c
arch/ia64/kernel/ptrace.c
arch/m32r/kernel/ptrace.c
arch/mips/kernel/ptrace32.c
arch/powerpc/kernel/ptrace32.c
arch/score/kernel/ptrace.c
arch/sparc/kernel/ptrace_64.c
arch/x86/kernel/step.c
arch/x86/um/ptrace_32.c
arch/x86/um/ptrace_64.c
include/linux/mm.h
kernel/ptrace.c
mm/memory.c
mm/nommu.c
mm/util.c

index d9ee817..940dfb4 100644 (file)
@@ -157,14 +157,16 @@ put_reg(struct task_struct *task, unsigned long regno, unsigned long data)
 static inline int
 read_int(struct task_struct *task, unsigned long addr, int * data)
 {
-       int copied = access_process_vm(task, addr, data, sizeof(int), 0);
+       int copied = access_process_vm(task, addr, data, sizeof(int),
+                       FOLL_FORCE);
        return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
 static inline int
 write_int(struct task_struct *task, unsigned long addr, int data)
 {
-       int copied = access_process_vm(task, addr, &data, sizeof(int), 1);
+       int copied = access_process_vm(task, addr, &data, sizeof(int),
+                       FOLL_FORCE | FOLL_WRITE);
        return (copied == sizeof(int)) ? 0 : -EIO;
 }
 
@@ -281,7 +283,8 @@ 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 = access_process_vm(child, addr, &tmp, sizeof(tmp),
+                               FOLL_FORCE);
                ret = -EIO;
                if (copied != sizeof(tmp))
                        break;
index 8b8fe67..8d79286 100644 (file)
@@ -271,7 +271,7 @@ long arch_ptrace(struct task_struct *child, long request,
                        case BFIN_MEM_ACCESS_CORE:
                        case BFIN_MEM_ACCESS_CORE_ONLY:
                                copied = access_process_vm(child, addr, &tmp,
-                                                          to_copy, 0);
+                                                          to_copy, FOLL_FORCE);
                                if (copied)
                                        break;
 
@@ -324,7 +324,8 @@ long arch_ptrace(struct task_struct *child, long request,
                        case BFIN_MEM_ACCESS_CORE:
                        case BFIN_MEM_ACCESS_CORE_ONLY:
                                copied = access_process_vm(child, addr, &data,
-                                                          to_copy, 1);
+                                                          to_copy,
+                                                          FOLL_FORCE | FOLL_WRITE);
                                break;
                        case BFIN_MEM_ACCESS_DMA:
                                if (safe_dma_memcpy(paddr, &data, to_copy))
index f085229..f0df654 100644 (file)
@@ -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 = access_process_vm(child, addr, &tmp, sizeof(tmp), FOLL_FORCE);
 
                                if (copied != sizeof(tmp))
                                        break;
@@ -279,7 +279,7 @@ static int insn_size(struct task_struct *child, unsigned long pc)
   int opsize = 0;
 
   /* Read the opcode at pc (do what PTRACE_PEEKTEXT would do). */
-  copied = access_process_vm(child, pc, &opcode, sizeof(opcode), 0);
+  copied = access_process_vm(child, pc, &opcode, sizeof(opcode), FOLL_FORCE);
   if (copied != sizeof(opcode))
     return 0;
 
index 6f54d51..31aa8c0 100644 (file)
@@ -453,7 +453,7 @@ ia64_peek (struct task_struct *child, struct switch_stack *child_stack,
                        return 0;
                }
        }
-       copied = access_process_vm(child, addr, &ret, sizeof(ret), 0);
+       copied = access_process_vm(child, addr, &ret, sizeof(ret), FOLL_FORCE);
        if (copied != sizeof(ret))
                return -EIO;
        *val = ret;
@@ -489,7 +489,8 @@ ia64_poke (struct task_struct *child, struct switch_stack *child_stack,
                                *ia64_rse_skip_regs(krbs, regnum) = val;
                        }
                }
-       } else if (access_process_vm(child, addr, &val, sizeof(val), 1)
+       } else if (access_process_vm(child, addr, &val, sizeof(val),
+                               FOLL_FORCE | FOLL_WRITE)
                   != sizeof(val))
                return -EIO;
        return 0;
@@ -543,7 +544,8 @@ ia64_sync_user_rbs (struct task_struct *child, struct switch_stack *sw,
                ret = ia64_peek(child, sw, user_rbs_end, addr, &val);
                if (ret < 0)
                        return ret;
-               if (access_process_vm(child, addr, &val, sizeof(val), 1)
+               if (access_process_vm(child, addr, &val, sizeof(val),
+                               FOLL_FORCE | FOLL_WRITE)
                    != sizeof(val))
                        return -EIO;
        }
@@ -559,7 +561,8 @@ ia64_sync_kernel_rbs (struct task_struct *child, struct switch_stack *sw,
 
        /* now copy word for word from user rbs to kernel rbs: */
        for (addr = user_rbs_start; addr < user_rbs_end; addr += 8) {
-               if (access_process_vm(child, addr, &val, sizeof(val), 0)
+               if (access_process_vm(child, addr, &val, sizeof(val),
+                               FOLL_FORCE)
                                != sizeof(val))
                        return -EIO;
 
@@ -1156,7 +1159,8 @@ 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 (access_process_vm(child, addr, &data, sizeof(data),
+                               FOLL_FORCE)
                    != sizeof(data))
                        return -EIO;
                /* ensure return value is not mistaken for error code */
index 51f5e9a..c145605 100644 (file)
@@ -493,7 +493,8 @@ unregister_all_debug_traps(struct task_struct *child)
        int i;
 
        for (i = 0; i < p->nr_trap; i++)
-               access_process_vm(child, p->addr[i], &p->insn[i], sizeof(p->insn[i]), 1);
+               access_process_vm(child, p->addr[i], &p->insn[i], sizeof(p->insn[i]),
+                               FOLL_FORCE | FOLL_WRITE);
        p->nr_trap = 0;
 }
 
@@ -537,7 +538,8 @@ embed_debug_trap(struct task_struct *child, unsigned long next_pc)
        unsigned long next_insn, code;
        unsigned long addr = next_pc & ~3;
 
-       if (access_process_vm(child, addr, &next_insn, sizeof(next_insn), 0)
+       if (access_process_vm(child, addr, &next_insn, sizeof(next_insn),
+                       FOLL_FORCE)
            != sizeof(next_insn)) {
                return -1; /* error */
        }
@@ -546,7 +548,8 @@ embed_debug_trap(struct task_struct *child, unsigned long next_pc)
        if (register_debug_trap(child, next_pc, next_insn, &code)) {
                return -1; /* error */
        }
-       if (access_process_vm(child, addr, &code, sizeof(code), 1)
+       if (access_process_vm(child, addr, &code, sizeof(code),
+                       FOLL_FORCE | FOLL_WRITE)
            != sizeof(code)) {
                return -1; /* error */
        }
@@ -562,7 +565,8 @@ withdraw_debug_trap(struct pt_regs *regs)
        addr = (regs->bpc - 2) & ~3;
        regs->bpc -= 2;
        if (unregister_debug_trap(current, addr, &code)) {
-           access_process_vm(current, addr, &code, sizeof(code), 1);
+           access_process_vm(current, addr, &code, sizeof(code),
+                   FOLL_FORCE | FOLL_WRITE);
            invalidate_cache();
        }
 }
@@ -589,7 +593,8 @@ void user_enable_single_step(struct task_struct *child)
        /* Compute next pc.  */
        pc = get_stack_long(child, PT_BPC);
 
-       if (access_process_vm(child, pc&~3, &insn, sizeof(insn), 0)
+       if (access_process_vm(child, pc&~3, &insn, sizeof(insn),
+                       FOLL_FORCE)
            != sizeof(insn))
                return;
 
index 283b5a1..7e71a4e 100644 (file)
@@ -70,7 +70,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                        break;
 
                copied = access_process_vm(child, (u64)addrOthers, &tmp,
-                               sizeof(tmp), 0);
+                               sizeof(tmp), FOLL_FORCE);
                if (copied != sizeof(tmp))
                        break;
                ret = put_user(tmp, (u32 __user *) (unsigned long) data);
@@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                        break;
                ret = 0;
                if (access_process_vm(child, (u64)addrOthers, &data,
-                                       sizeof(data), 1) == sizeof(data))
+                                       sizeof(data),
+                                       FOLL_FORCE | FOLL_WRITE) == sizeof(data))
                        break;
                ret = -EIO;
                break;
index f52b7db..010b7b3 100644 (file)
@@ -74,7 +74,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                        break;
 
                copied = access_process_vm(child, (u64)addrOthers, &tmp,
-                               sizeof(tmp), 0);
+                               sizeof(tmp), FOLL_FORCE);
                if (copied != sizeof(tmp))
                        break;
                ret = put_user(tmp, (u32 __user *)data);
@@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
                        break;
                ret = 0;
                if (access_process_vm(child, (u64)addrOthers, &tmp,
-                                       sizeof(tmp), 1) == sizeof(tmp))
+                                       sizeof(tmp),
+                                       FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
                        break;
                ret = -EIO;
                break;
index 5583618..4f7314d 100644 (file)
@@ -131,7 +131,7 @@ read_tsk_long(struct task_struct *child,
 {
        int copied;
 
-       copied = access_process_vm(child, addr, res, sizeof(*res), 0);
+       copied = access_process_vm(child, addr, res, sizeof(*res), FOLL_FORCE);
 
        return copied != sizeof(*res) ? -EIO : 0;
 }
@@ -142,7 +142,7 @@ read_tsk_short(struct task_struct *child,
 {
        int copied;
 
-       copied = access_process_vm(child, addr, res, sizeof(*res), 0);
+       copied = access_process_vm(child, addr, res, sizeof(*res), FOLL_FORCE);
 
        return copied != sizeof(*res) ? -EIO : 0;
 }
@@ -153,7 +153,8 @@ write_tsk_short(struct task_struct *child,
 {
        int copied;
 
-       copied = access_process_vm(child, addr, &val, sizeof(val), 1);
+       copied = access_process_vm(child, addr, &val, sizeof(val),
+                       FOLL_FORCE | FOLL_WRITE);
 
        return copied != sizeof(val) ? -EIO : 0;
 }
@@ -164,7 +165,8 @@ write_tsk_long(struct task_struct *child,
 {
        int copied;
 
-       copied = access_process_vm(child, addr, &val, sizeof(val), 1);
+       copied = access_process_vm(child, addr, &val, sizeof(val),
+                       FOLL_FORCE | FOLL_WRITE);
 
        return copied != sizeof(val) ? -EIO : 0;
 }
index 9ddc492..ac082dd 100644 (file)
@@ -127,7 +127,8 @@ static int get_from_target(struct task_struct *target, unsigned long uaddr,
                if (copy_from_user(kbuf, (void __user *) uaddr, len))
                        return -EFAULT;
        } else {
-               int len2 = access_process_vm(target, uaddr, kbuf, len, 0);
+               int len2 = access_process_vm(target, uaddr, kbuf, len,
+                               FOLL_FORCE);
                if (len2 != len)
                        return -EFAULT;
        }
@@ -141,7 +142,8 @@ static int set_to_target(struct task_struct *target, unsigned long uaddr,
                if (copy_to_user((void __user *) uaddr, kbuf, len))
                        return -EFAULT;
        } else {
-               int len2 = access_process_vm(target, uaddr, kbuf, len, 1);
+               int len2 = access_process_vm(target, uaddr, kbuf, len,
+                               FOLL_FORCE | FOLL_WRITE);
                if (len2 != len)
                        return -EFAULT;
        }
@@ -505,7 +507,8 @@ static int genregs32_get(struct task_struct *target,
                                if (access_process_vm(target,
                                                      (unsigned long)
                                                      &reg_window[pos],
-                                                     k, sizeof(*k), 0)
+                                                     k, sizeof(*k),
+                                                     FOLL_FORCE)
                                    != sizeof(*k))
                                        return -EFAULT;
                                k++;
@@ -531,12 +534,14 @@ static int genregs32_get(struct task_struct *target,
                                if (access_process_vm(target,
                                                      (unsigned long)
                                                      &reg_window[pos],
-                                                     &reg, sizeof(reg), 0)
+                                                     &reg, sizeof(reg),
+                                                     FOLL_FORCE)
                                    != sizeof(reg))
                                        return -EFAULT;
                                if (access_process_vm(target,
                                                      (unsigned long) u,
-                                                     &reg, sizeof(reg), 1)
+                                                     &reg, sizeof(reg),
+                                                     FOLL_FORCE | FOLL_WRITE)
                                    != sizeof(reg))
                                        return -EFAULT;
                                pos++;
@@ -615,7 +620,8 @@ static int genregs32_set(struct task_struct *target,
                                                      (unsigned long)
                                                      &reg_window[pos],
                                                      (void *) k,
-                                                     sizeof(*k), 1)
+                                                     sizeof(*k),
+                                                     FOLL_FORCE | FOLL_WRITE)
                                    != sizeof(*k))
                                        return -EFAULT;
                                k++;
@@ -642,13 +648,15 @@ static int genregs32_set(struct task_struct *target,
                                if (access_process_vm(target,
                                                      (unsigned long)
                                                      u,
-                                                     &reg, sizeof(reg), 0)
+                                                     &reg, sizeof(reg),
+                                                     FOLL_FORCE)
                                    != sizeof(reg))
                                        return -EFAULT;
                                if (access_process_vm(target,
                                                      (unsigned long)
                                                      &reg_window[pos],
-                                                     &reg, sizeof(reg), 1)
+                                                     &reg, sizeof(reg),
+                                                     FOLL_FORCE | FOLL_WRITE)
                                    != sizeof(reg))
                                        return -EFAULT;
                                pos++;
index c9a0738..a23ce84 100644 (file)
@@ -57,7 +57,8 @@ static int is_setting_trap_flag(struct task_struct *child, struct pt_regs *regs)
        unsigned char opcode[15];
        unsigned long addr = convert_ip_to_linear(child, regs);
 
-       copied = access_process_vm(child, addr, opcode, sizeof(opcode), 0);
+       copied = access_process_vm(child, addr, opcode, sizeof(opcode),
+                       FOLL_FORCE);
        for (i = 0; i < copied; i++) {
                switch (opcode[i]) {
                /* popf and iret */
index 5766ead..60a5a5a 100644 (file)
@@ -36,7 +36,8 @@ int is_syscall(unsigned long addr)
                 * slow, but that doesn't matter, since it will be called only
                 * in case of singlestepping, if copy_from_user failed.
                 */
-               n = access_process_vm(current, addr, &instr, sizeof(instr), 0);
+               n = access_process_vm(current, addr, &instr, sizeof(instr),
+                               FOLL_FORCE);
                if (n != sizeof(instr)) {
                        printk(KERN_ERR "is_syscall : failed to read "
                               "instruction from 0x%lx\n", addr);
index 0b5c184..e30202b 100644 (file)
@@ -212,7 +212,8 @@ int is_syscall(unsigned long addr)
                 * slow, but that doesn't matter, since it will be called only
                 * in case of singlestepping, if copy_from_user failed.
                 */
-               n = access_process_vm(current, addr, &instr, sizeof(instr), 0);
+               n = access_process_vm(current, addr, &instr, sizeof(instr),
+                               FOLL_FORCE);
                if (n != sizeof(instr)) {
                        printk("is_syscall : failed to read instruction from "
                               "0x%lx\n", addr);
index f31bf90..ffbd729 100644 (file)
@@ -1266,7 +1266,8 @@ static inline int fixup_user_fault(struct task_struct *tsk,
 }
 #endif
 
-extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write);
+extern int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len,
+               unsigned int gup_flags);
 extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
                void *buf, int len, unsigned int gup_flags);
 
index 2a99027..e6474f7 100644 (file)
@@ -537,7 +537,7 @@ 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 = access_process_vm(tsk, src, buf, this_len, FOLL_FORCE);
                if (!retval) {
                        if (copied)
                                break;
@@ -564,7 +564,8 @@ 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 = access_process_vm(tsk, dst, buf, this_len,
+                               FOLL_FORCE | FOLL_WRITE);
                if (!retval) {
                        if (copied)
                                break;
@@ -1127,7 +1128,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 = access_process_vm(tsk, addr, &tmp, sizeof(tmp), FOLL_FORCE);
        if (copied != sizeof(tmp))
                return -EIO;
        return put_user(tmp, (unsigned long __user *)data);
@@ -1138,7 +1139,8 @@ int generic_ptrace_pokedata(struct task_struct *tsk, unsigned long addr,
 {
        int copied;
 
-       copied = access_process_vm(tsk, addr, &data, sizeof(data), 1);
+       copied = access_process_vm(tsk, addr, &data, sizeof(data),
+                       FOLL_FORCE | FOLL_WRITE);
        return (copied == sizeof(data)) ? 0 : -EIO;
 }
 
@@ -1155,7 +1157,8 @@ 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 = access_process_vm(child, addr, &word, sizeof(word),
+                               FOLL_FORCE);
                if (ret != sizeof(word))
                        ret = -EIO;
                else
@@ -1164,7 +1167,8 @@ 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 = access_process_vm(child, addr, &data, sizeof(data),
+                               FOLL_FORCE | FOLL_WRITE);
                ret = (ret != sizeof(data) ? -EIO : 0);
                break;
 
index bac2d99..e18c57b 100644 (file)
@@ -3951,20 +3951,16 @@ int access_remote_vm(struct mm_struct *mm, unsigned long addr,
  * Do not walk the page table directly, use get_user_pages
  */
 int access_process_vm(struct task_struct *tsk, unsigned long addr,
-               void *buf, int len, int write)
+               void *buf, int len, unsigned int gup_flags)
 {
        struct mm_struct *mm;
        int ret;
-       unsigned int flags = FOLL_FORCE;
 
        mm = get_task_mm(tsk);
        if (!mm)
                return 0;
 
-       if (write)
-               flags |= FOLL_WRITE;
-
-       ret = __access_remote_vm(tsk, mm, addr, buf, len, flags);
+       ret = __access_remote_vm(tsk, mm, addr, buf, len, gup_flags);
 
        mmput(mm);
 
index 93d5bb5..db5fd17 100644 (file)
@@ -1861,7 +1861,8 @@ int access_remote_vm(struct mm_struct *mm, unsigned long addr,
  * Access another process' address space.
  * - source/target buffer must be kernel space
  */
-int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write)
+int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len,
+               unsigned int gup_flags)
 {
        struct mm_struct *mm;
 
@@ -1872,8 +1873,7 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, in
        if (!mm)
                return 0;
 
-       len = __access_remote_vm(tsk, mm, addr, buf, len,
-                       write ? FOLL_WRITE : 0);
+       len = __access_remote_vm(tsk, mm, addr, buf, len, gup_flags);
 
        mmput(mm);
        return len;
index 4c685bd..952cbe7 100644 (file)
--- a/mm/util.c
+++ b/mm/util.c
@@ -624,7 +624,7 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen)
        if (len > buflen)
                len = buflen;
 
-       res = access_process_vm(task, arg_start, buffer, len, 0);
+       res = access_process_vm(task, arg_start, buffer, len, FOLL_FORCE);
 
        /*
         * If the nul at the end of args has been overwritten, then
@@ -639,7 +639,8 @@ int get_cmdline(struct task_struct *task, char *buffer, int buflen)
                        if (len > buflen - res)
                                len = buflen - res;
                        res += access_process_vm(task, env_start,
-                                                buffer+res, len, 0);
+                                                buffer+res, len,
+                                                FOLL_FORCE);
                        res = strnlen(buffer, res);
                }
        }