commit fa5713f5b0214732cd02a8d155daa2077bffbfa0 Author: Tejun Heo Date: Thu Jul 3 15:43:15 2014 -0400 Upstream commit: b9cd18de4db3c9ffa7e17b0dc0ca99ed5aa4d43a ptrace,x86: force IRET path after a ptrace_stop() The 'sysret' fastpath does not correctly restore even all regular registers, much less any segment registers or reflags values. That is very much part of why it's faster than 'iret'. Normally that isn't a problem, because the normal ptrace() interface catches the process using the signal handler infrastructure, which always returns with an iret. However, some paths can get caught using ptrace_event() instead of the signal path, and for those we need to make sure that we aren't going to return to user space using 'sysret'. Otherwise the modifications that may have been done to the register set by the tracer wouldn't necessarily take effect. Fix it by forcing IRET path by setting TIF_NOTIFY_RESUME from arch_ptrace_stop_needed() which is invoked from ptrace_stop(). Signed-off-by: Tejun Heo Reported-by: Andy Lutomirski Acked-by: Oleg Nesterov Suggested-by: Linus Torvalds Cc: stable@vger.kernel.org Signed-off-by: Linus Torvalds commit f302b21ee907f65af6434d8618d2c1a921c02d48 Author: Andy Lutomirski Date: Wed Jul 2 14:52:55 2014 -0700 x86_64,ptrace: Enforce RIP <= TASK_SIZE_MAX (CVE-2014-4699) Don't allow ptrace to set RIP to a value that couldn't happen by ordinary control flow. There are CPU bugs^Wfeatures that can have interesting effects if RIP is non-canonical. I didn't make the corresponding x86_32 change, since x86_32 has no concept of canonical addresses. putreg32 doesn't need this fix: value is only 32 bits, so it can't be non-canonical. Fixes CVE-2014-4699. There are arguably still bugs here, but this fixes the major issue. Signed-off-by: Andy Lutomirski CVE-2014-4699 BugLink: http://bugs.launchpad.net/bugs/1337339 Acked-by: Andy Whitcroft Signed-off-by: John Johansen Signed-off-by: Luis Henriques diff --git a/arch/x86/include/asm/ptrace.h b/arch/x86/include/asm/ptrace.h index 8790004..d8c7f13 100644 --- a/arch/x86/include/asm/ptrace.h +++ b/arch/x86/include/asm/ptrace.h @@ -289,6 +289,22 @@ static inline unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, #define ARCH_HAS_USER_SINGLE_STEP_INFO +/* + * When hitting ptrace_stop(), we cannot return using SYSRET because + * that does not restore the full CPU state, only a minimal set. The + * ptracer can change arbitrary register values, which is usually okay + * because the usual ptrace stops run off the signal delivery path which + * forces IRET; however, ptrace_event() stops happen in arbitrary places + * in the kernel and don't force IRET path. + * + * So force IRET path after a ptrace stop. + */ +#define arch_ptrace_stop_needed(code, info) \ +({ \ + set_thread_flag(TIF_NOTIFY_RESUME); \ + false; \ +}) + struct user_desc; extern int do_get_thread_area(struct task_struct *p, int idx, struct user_desc __user *info); diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index 13b3715..635a20c 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -377,6 +377,9 @@ static inline void user_single_step_siginfo(struct task_struct *tsk, * calling arch_ptrace_stop() when it would be superfluous. For example, * if the thread has not been back to user mode since the last stop, the * thread state might indicate that nothing needs to be done. + * + * This is guaranteed to be invoked once before a task stops for ptrace and + * may include arch-specific operations necessary prior to a ptrace stop. */ #define arch_ptrace_stop_needed(code, info) (0) #endif diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c index 8659c9e..011d41e 100644 --- a/arch/x86/kernel/ptrace.c +++ b/arch/x86/kernel/ptrace.c @@ -449,6 +449,20 @@ static int putreg(struct task_struct *child, if (child->thread.gs != value) return do_arch_prctl(child, ARCH_SET_GS, value); return 0; + + case offsetof(struct user_regs_struct,ip): + /* + * Protect against any attempt to set ip to an + * impossible address. There are dragons lurking if the + * address is noncanonical. (This explicitly allows + * setting ip to TASK_SIZE_MAX, because user code can do + * that all by itself by running off the end of its + * address space. + */ + if (value > TASK_SIZE_MAX) + return -EIO; + break; + #endif }