Skip to content

Commit

Permalink
mm: replace access_process_vm() write parameter with gup_flags
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@lorenzo-stoakes @torvalds
lorenzo-stoakes authored and torvalds committed Oct 19, 2016
1 parent 6347e8d commit f307ab6
Show file tree
Hide file tree
Showing 17 changed files with 84 additions and 54 deletions.
9 changes: 6 additions & 3 deletions arch/alpha/kernel/ptrace.c
View file
Expand Up @@ -157,14 +157,16 @@ put_reg(struct task_struct *task, unsigned long regno, unsigned long data)
static inline int static inline int
read_int(struct task_struct *task, unsigned long addr, int * data) 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; return (copied == sizeof(int)) ? 0 : -EIO;
} }


static inline int static inline int
write_int(struct task_struct *task, unsigned long addr, int data) 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; return (copied == sizeof(int)) ? 0 : -EIO;
} }


Expand Down Expand Up @@ -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. */ /* When I and D space are separate, these will need to be fixed. */
case PTRACE_PEEKTEXT: /* read word at location addr. */ case PTRACE_PEEKTEXT: /* read word at location addr. */
case PTRACE_PEEKDATA: 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; ret = -EIO;
if (copied != sizeof(tmp)) if (copied != sizeof(tmp))
break; break;
Expand Down
5 changes: 3 additions & 2 deletions arch/blackfin/kernel/ptrace.c
View file
Expand Up @@ -271,7 +271,7 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE: case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY: case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, &tmp, copied = access_process_vm(child, addr, &tmp,
to_copy, 0); to_copy, FOLL_FORCE);
if (copied) if (copied)
break; break;


Expand Down Expand Up @@ -324,7 +324,8 @@ long arch_ptrace(struct task_struct *child, long request,
case BFIN_MEM_ACCESS_CORE: case BFIN_MEM_ACCESS_CORE:
case BFIN_MEM_ACCESS_CORE_ONLY: case BFIN_MEM_ACCESS_CORE_ONLY:
copied = access_process_vm(child, addr, &data, copied = access_process_vm(child, addr, &data,
to_copy, 1); to_copy,
FOLL_FORCE | FOLL_WRITE);
break; break;
case BFIN_MEM_ACCESS_DMA: case BFIN_MEM_ACCESS_DMA:
if (safe_dma_memcpy(paddr, &data, to_copy)) if (safe_dma_memcpy(paddr, &data, to_copy))
Expand Down
4 changes: 2 additions & 2 deletions arch/cris/arch-v32/kernel/ptrace.c
View file
Expand Up @@ -147,7 +147,7 @@ long arch_ptrace(struct task_struct *child, long request,
/* The trampoline page is globally mapped, no page table to traverse.*/ /* The trampoline page is globally mapped, no page table to traverse.*/
tmp = *(unsigned long*)addr; tmp = *(unsigned long*)addr;
} else { } 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)) if (copied != sizeof(tmp))
break; break;
Expand Down Expand Up @@ -279,7 +279,7 @@ static int insn_size(struct task_struct *child, unsigned long pc)
int opsize = 0; int opsize = 0;


/* Read the opcode at pc (do what PTRACE_PEEKTEXT would do). */ /* 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)) if (copied != sizeof(opcode))
return 0; return 0;


Expand Down
14 changes: 9 additions & 5 deletions arch/ia64/kernel/ptrace.c
View file
Expand Up @@ -453,7 +453,7 @@ ia64_peek (struct task_struct *child, struct switch_stack *child_stack,
return 0; 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)) if (copied != sizeof(ret))
return -EIO; return -EIO;
*val = ret; *val = ret;
Expand Down Expand Up @@ -489,7 +489,8 @@ ia64_poke (struct task_struct *child, struct switch_stack *child_stack,
*ia64_rse_skip_regs(krbs, regnum) = val; *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)) != sizeof(val))
return -EIO; return -EIO;
return 0; return 0;
Expand Down Expand Up @@ -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); ret = ia64_peek(child, sw, user_rbs_end, addr, &val);
if (ret < 0) if (ret < 0)
return ret; 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)) != sizeof(val))
return -EIO; return -EIO;
} }
Expand All @@ -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: */ /* now copy word for word from user rbs to kernel rbs: */
for (addr = user_rbs_start; addr < user_rbs_end; addr += 8) { 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)) != sizeof(val))
return -EIO; return -EIO;


Expand Down Expand Up @@ -1156,7 +1159,8 @@ arch_ptrace (struct task_struct *child, long request,
case PTRACE_PEEKTEXT: case PTRACE_PEEKTEXT:
case PTRACE_PEEKDATA: case PTRACE_PEEKDATA:
/* read word at location addr */ /* 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)) != sizeof(data))
return -EIO; return -EIO;
/* ensure return value is not mistaken for error code */ /* ensure return value is not mistaken for error code */
Expand Down
15 changes: 10 additions & 5 deletions arch/m32r/kernel/ptrace.c
View file
Expand Up @@ -493,7 +493,8 @@ unregister_all_debug_traps(struct task_struct *child)
int i; int i;


for (i = 0; i < p->nr_trap; 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; p->nr_trap = 0;
} }


Expand Down Expand Up @@ -537,7 +538,8 @@ embed_debug_trap(struct task_struct *child, unsigned long next_pc)
unsigned long next_insn, code; unsigned long next_insn, code;
unsigned long addr = next_pc & ~3; 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)) { != sizeof(next_insn)) {
return -1; /* error */ return -1; /* error */
} }
Expand All @@ -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)) { if (register_debug_trap(child, next_pc, next_insn, &code)) {
return -1; /* error */ 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)) { != sizeof(code)) {
return -1; /* error */ return -1; /* error */
} }
Expand All @@ -562,7 +565,8 @@ withdraw_debug_trap(struct pt_regs *regs)
addr = (regs->bpc - 2) & ~3; addr = (regs->bpc - 2) & ~3;
regs->bpc -= 2; regs->bpc -= 2;
if (unregister_debug_trap(current, addr, &code)) { 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(); invalidate_cache();
} }
} }
Expand All @@ -589,7 +593,8 @@ void user_enable_single_step(struct task_struct *child)
/* Compute next pc. */ /* Compute next pc. */
pc = get_stack_long(child, PT_BPC); 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)) != sizeof(insn))
return; return;


Expand Down
5 changes: 3 additions & 2 deletions arch/mips/kernel/ptrace32.c
View file
Expand Up @@ -70,7 +70,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
break; break;


copied = access_process_vm(child, (u64)addrOthers, &tmp, copied = access_process_vm(child, (u64)addrOthers, &tmp,
sizeof(tmp), 0); sizeof(tmp), FOLL_FORCE);
if (copied != sizeof(tmp)) if (copied != sizeof(tmp))
break; break;
ret = put_user(tmp, (u32 __user *) (unsigned long) data); ret = put_user(tmp, (u32 __user *) (unsigned long) data);
Expand Down Expand Up @@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
break; break;
ret = 0; ret = 0;
if (access_process_vm(child, (u64)addrOthers, &data, if (access_process_vm(child, (u64)addrOthers, &data,
sizeof(data), 1) == sizeof(data)) sizeof(data),
FOLL_FORCE | FOLL_WRITE) == sizeof(data))
break; break;
ret = -EIO; ret = -EIO;
break; break;
Expand Down
5 changes: 3 additions & 2 deletions arch/powerpc/kernel/ptrace32.c
View file
Expand Up @@ -74,7 +74,7 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
break; break;


copied = access_process_vm(child, (u64)addrOthers, &tmp, copied = access_process_vm(child, (u64)addrOthers, &tmp,
sizeof(tmp), 0); sizeof(tmp), FOLL_FORCE);
if (copied != sizeof(tmp)) if (copied != sizeof(tmp))
break; break;
ret = put_user(tmp, (u32 __user *)data); ret = put_user(tmp, (u32 __user *)data);
Expand Down Expand Up @@ -179,7 +179,8 @@ long compat_arch_ptrace(struct task_struct *child, compat_long_t request,
break; break;
ret = 0; ret = 0;
if (access_process_vm(child, (u64)addrOthers, &tmp, if (access_process_vm(child, (u64)addrOthers, &tmp,
sizeof(tmp), 1) == sizeof(tmp)) sizeof(tmp),
FOLL_FORCE | FOLL_WRITE) == sizeof(tmp))
break; break;
ret = -EIO; ret = -EIO;
break; break;
Expand Down
10 changes: 6 additions & 4 deletions arch/score/kernel/ptrace.c
View file
Expand Up @@ -131,7 +131,7 @@ read_tsk_long(struct task_struct *child,
{ {
int copied; 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; return copied != sizeof(*res) ? -EIO : 0;
} }
Expand All @@ -142,7 +142,7 @@ read_tsk_short(struct task_struct *child,
{ {
int copied; 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; return copied != sizeof(*res) ? -EIO : 0;
} }
Expand All @@ -153,7 +153,8 @@ write_tsk_short(struct task_struct *child,
{ {
int copied; 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; return copied != sizeof(val) ? -EIO : 0;
} }
Expand All @@ -164,7 +165,8 @@ write_tsk_long(struct task_struct *child,
{ {
int copied; 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; return copied != sizeof(val) ? -EIO : 0;
} }
Expand Down
24 changes: 16 additions & 8 deletions arch/sparc/kernel/ptrace_64.c
View file
Expand Up @@ -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)) if (copy_from_user(kbuf, (void __user *) uaddr, len))
return -EFAULT; return -EFAULT;
} else { } 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) if (len2 != len)
return -EFAULT; return -EFAULT;
} }
Expand All @@ -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)) if (copy_to_user((void __user *) uaddr, kbuf, len))
return -EFAULT; return -EFAULT;
} else { } 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) if (len2 != len)
return -EFAULT; return -EFAULT;
} }
Expand Down Expand Up @@ -505,7 +507,8 @@ static int genregs32_get(struct task_struct *target,
if (access_process_vm(target, if (access_process_vm(target,
(unsigned long) (unsigned long)
&reg_window[pos], &reg_window[pos],
k, sizeof(*k), 0) k, sizeof(*k),
FOLL_FORCE)
!= sizeof(*k)) != sizeof(*k))
return -EFAULT; return -EFAULT;
k++; k++;
Expand All @@ -531,12 +534,14 @@ static int genregs32_get(struct task_struct *target,
if (access_process_vm(target, if (access_process_vm(target,
(unsigned long) (unsigned long)
&reg_window[pos], &reg_window[pos],
&reg, sizeof(reg), 0) &reg, sizeof(reg),
FOLL_FORCE)
!= sizeof(reg)) != sizeof(reg))
return -EFAULT; return -EFAULT;
if (access_process_vm(target, if (access_process_vm(target,
(unsigned long) u, (unsigned long) u,
&reg, sizeof(reg), 1) &reg, sizeof(reg),
FOLL_FORCE | FOLL_WRITE)
!= sizeof(reg)) != sizeof(reg))
return -EFAULT; return -EFAULT;
pos++; pos++;
Expand Down Expand Up @@ -615,7 +620,8 @@ static int genregs32_set(struct task_struct *target,
(unsigned long) (unsigned long)
&reg_window[pos], &reg_window[pos],
(void *) k, (void *) k,
sizeof(*k), 1) sizeof(*k),
FOLL_FORCE | FOLL_WRITE)
!= sizeof(*k)) != sizeof(*k))
return -EFAULT; return -EFAULT;
k++; k++;
Expand All @@ -642,13 +648,15 @@ static int genregs32_set(struct task_struct *target,
if (access_process_vm(target, if (access_process_vm(target,
(unsigned long) (unsigned long)
u, u,
&reg, sizeof(reg), 0) &reg, sizeof(reg),
FOLL_FORCE)
!= sizeof(reg)) != sizeof(reg))
return -EFAULT; return -EFAULT;
if (access_process_vm(target, if (access_process_vm(target,
(unsigned long) (unsigned long)
&reg_window[pos], &reg_window[pos],
&reg, sizeof(reg), 1) &reg, sizeof(reg),
FOLL_FORCE | FOLL_WRITE)
!= sizeof(reg)) != sizeof(reg))
return -EFAULT; return -EFAULT;
pos++; pos++;
Expand Down
3 changes: 2 additions & 1 deletion arch/x86/kernel/step.c
View file
Expand Up @@ -57,7 +57,8 @@ static int is_setting_trap_flag(struct task_struct *child, struct pt_regs *regs)
unsigned char opcode[15]; unsigned char opcode[15];
unsigned long addr = convert_ip_to_linear(child, regs); 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++) { for (i = 0; i < copied; i++) {
switch (opcode[i]) { switch (opcode[i]) {
/* popf and iret */ /* popf and iret */
Expand Down
3 changes: 2 additions & 1 deletion arch/x86/um/ptrace_32.c
View file
Expand Up @@ -36,7 +36,8 @@ int is_syscall(unsigned long addr)
* slow, but that doesn't matter, since it will be called only * slow, but that doesn't matter, since it will be called only
* in case of singlestepping, if copy_from_user failed. * 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)) { if (n != sizeof(instr)) {
printk(KERN_ERR "is_syscall : failed to read " printk(KERN_ERR "is_syscall : failed to read "
"instruction from 0x%lx\n", addr); "instruction from 0x%lx\n", addr);
Expand Down
3 changes: 2 additions & 1 deletion arch/x86/um/ptrace_64.c
View file
Expand Up @@ -212,7 +212,8 @@ int is_syscall(unsigned long addr)
* slow, but that doesn't matter, since it will be called only * slow, but that doesn't matter, since it will be called only
* in case of singlestepping, if copy_from_user failed. * 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)) { if (n != sizeof(instr)) {
printk("is_syscall : failed to read instruction from " printk("is_syscall : failed to read instruction from "
"0x%lx\n", addr); "0x%lx\n", addr);
Expand Down
3 changes: 2 additions & 1 deletion include/linux/mm.h
View file
Expand Up @@ -1266,7 +1266,8 @@ static inline int fixup_user_fault(struct task_struct *tsk,
} }
#endif #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, extern int access_remote_vm(struct mm_struct *mm, unsigned long addr,
void *buf, int len, unsigned int gup_flags); void *buf, int len, unsigned int gup_flags);


Expand Down

0 comments on commit f307ab6

Please sign in to comment.