From: Roland McGrath There is a race between PTRACE_ATTACH and the real parent calling wait. For a moment, the task is put in PT_PTRACED but with its parent still pointing to its real_parent. In this circumstance, if the real parent calls wait without the WUNTRACED flag, he can see a stopped child status, which wait should never return without WUNTRACED when the caller is not using ptrace. Here it is not the caller that is using ptrace, but some third party. This patch avoids this race condition by adding the PT_ATTACHED flag to distinguish a real parent from a ptrace_attach parent when PT_PTRACED is set, and then having wait use this flag to confirm that things are in order and not consider the child ptraced when its ->ptrace flags are set but its parent links have not yet been switched. (ptrace_check_attach also uses it similarly to rule out a possible race with a bogus ptrace call by the real parent during ptrace_attach.) While looking into this, I noticed that every arch's sys_execve has: current->ptrace &= ~PT_DTRACE; with no locking at all. So, if an exec happens in a race with PTRACE_ATTACH, you could wind up with ->ptrace not having PT_PTRACED set because this store clobbered it. That will cause later BUG hits because the parent links indicate ptracedness but the flag is not set. The patch corrects all the places I found to use task_lock around diddling ->ptrace when it's possible to be racing with ptrace_attach. (The ptrace operation code itself doesn't have this issue because it already excludes anyone else being in ptrace_attach.) Signed-off-by: Roland McGrath Signed-off-by: Andrew Morton --- 25-akpm/arch/i386/kernel/process.c | 2 ++ 25-akpm/arch/m32r/kernel/process.c | 5 ++++- 25-akpm/arch/parisc/hpux/fs.c | 5 ++++- 25-akpm/arch/parisc/kernel/process.c | 5 ++++- 25-akpm/arch/parisc/kernel/sys_parisc32.c | 5 ++++- 25-akpm/arch/ppc/kernel/process.c | 5 ++++- 25-akpm/arch/ppc64/kernel/process.c | 5 ++++- 25-akpm/arch/ppc64/kernel/sys_ppc32.c | 5 ++++- 25-akpm/arch/s390/kernel/compat_linux.c | 2 ++ 25-akpm/arch/s390/kernel/process.c | 2 ++ 25-akpm/arch/sh/kernel/process.c | 5 ++++- 25-akpm/arch/sh64/kernel/process.c | 5 ++++- 25-akpm/arch/sparc/kernel/process.c | 5 ++++- 25-akpm/arch/sparc64/kernel/process.c | 2 ++ 25-akpm/arch/sparc64/kernel/sys_sparc32.c | 2 ++ 25-akpm/arch/um/kernel/exec_kern.c | 2 ++ 25-akpm/arch/x86_64/ia32/sys_ia32.c | 5 ++++- 25-akpm/arch/x86_64/kernel/process.c | 5 ++++- 25-akpm/include/linux/ptrace.h | 1 + 25-akpm/kernel/exit.c | 20 ++++++++++++++++++-- 25-akpm/kernel/ptrace.c | 5 +++-- 21 files changed, 82 insertions(+), 16 deletions(-) diff -puN arch/i386/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/i386/kernel/process.c --- 25/arch/i386/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/i386/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -656,7 +656,9 @@ asmlinkage int sys_execve(struct pt_regs (char __user * __user *) regs.edx, ®s); if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); /* Make sure we don't return using sysenter.. */ set_thread_flag(TIF_IRET); } diff -puN arch/m32r/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/m32r/kernel/process.c --- 25/arch/m32r/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/m32r/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -335,8 +335,11 @@ asmlinkage int sys_execve(char __user *u goto out; error = do_execve(filename, uargv, uenvp, ®s); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: return error; diff -puN arch/parisc/hpux/fs.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/parisc/hpux/fs.c --- 25/arch/parisc/hpux/fs.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/parisc/hpux/fs.c Thu Sep 30 15:35:49 2004 @@ -43,8 +43,11 @@ int hpux_execve(struct pt_regs *regs) error = do_execve(filename, (char **) regs->gr[25], (char **)regs->gr[24], regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: diff -puN arch/parisc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/parisc/kernel/process.c --- 25/arch/parisc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/parisc/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -363,8 +363,11 @@ asmlinkage int sys_execve(struct pt_regs goto out; error = do_execve(filename, (char **) regs->gr[25], (char **) regs->gr[24], regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: diff -puN arch/parisc/kernel/sys_parisc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/parisc/kernel/sys_parisc32.c --- 25/arch/parisc/kernel/sys_parisc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/parisc/kernel/sys_parisc32.c Thu Sep 30 15:35:49 2004 @@ -80,8 +80,11 @@ asmlinkage int sys32_execve(struct pt_re goto out; error = compat_do_execve(filename, compat_ptr(regs->gr[25]), compat_ptr(regs->gr[24]), regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: diff -puN arch/ppc64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/ppc64/kernel/process.c --- 25/arch/ppc64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/ppc64/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -514,8 +514,11 @@ int sys_execve(unsigned long a0, unsigne error = do_execve(filename, (char __user * __user *) a1, (char __user * __user *) a2, regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: diff -puN arch/ppc64/kernel/sys_ppc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/ppc64/kernel/sys_ppc32.c --- 25/arch/ppc64/kernel/sys_ppc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/ppc64/kernel/sys_ppc32.c Thu Sep 30 15:35:49 2004 @@ -621,8 +621,11 @@ long sys32_execve(unsigned long a0, unsi error = compat_do_execve(filename, compat_ptr(a1), compat_ptr(a2), regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: diff -puN arch/ppc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/ppc/kernel/process.c --- 25/arch/ppc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/ppc/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -598,8 +598,11 @@ int sys_execve(unsigned long a0, unsigne preempt_enable(); error = do_execve(filename, (char __user *__user *) a1, (char __user *__user *) a2, regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: return error; diff -puN arch/s390/kernel/compat_linux.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/s390/kernel/compat_linux.c --- 25/arch/s390/kernel/compat_linux.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/s390/kernel/compat_linux.c Thu Sep 30 15:35:49 2004 @@ -751,7 +751,9 @@ sys32_execve(struct pt_regs regs) compat_ptr(regs.gprs[4]), ®s); if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); current->thread.fp_regs.fpc=0; __asm__ __volatile__ ("sr 0,0\n\t" diff -puN arch/s390/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/s390/kernel/process.c --- 25/arch/s390/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/s390/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -340,7 +340,9 @@ asmlinkage long sys_execve(struct pt_reg error = do_execve(filename, (char __user * __user *) regs.gprs[3], (char __user * __user *) regs.gprs[4], ®s); if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); current->thread.fp_regs.fpc = 0; if (MACHINE_HAS_IEEE) asm volatile("sfpc %0,%0" : : "d" (0)); diff -puN arch/sh64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/sh64/kernel/process.c --- 25/arch/sh64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/sh64/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -862,8 +862,11 @@ asmlinkage int sys_execve(char *ufilenam (char __user * __user *)uargv, (char __user * __user *)uenvp, pregs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: unlock_kernel(); diff -puN arch/sh/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/sh/kernel/process.c --- 25/arch/sh/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/sh/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -481,8 +481,11 @@ asmlinkage int sys_execve(char *ufilenam (char __user * __user *)uargv, (char __user * __user *)uenvp, ®s); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); out: return error; diff -puN arch/sparc64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/sparc64/kernel/process.c --- 25/arch/sparc64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/sparc64/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -829,7 +829,9 @@ asmlinkage int sparc_execve(struct pt_re current_thread_info()->xfsr[0] = 0; current_thread_info()->fpsaved[0] = 0; regs->tstate &= ~TSTATE_PEF; + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); } out: return error; diff -puN arch/sparc64/kernel/sys_sparc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/sparc64/kernel/sys_sparc32.c --- 25/arch/sparc64/kernel/sys_sparc32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/sparc64/kernel/sys_sparc32.c Thu Sep 30 15:35:49 2004 @@ -1274,7 +1274,9 @@ asmlinkage long sparc32_execve(struct pt current_thread_info()->xfsr[0] = 0; current_thread_info()->fpsaved[0] = 0; regs->tstate &= ~TSTATE_PEF; + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); } out: return error; diff -puN arch/sparc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/sparc/kernel/process.c --- 25/arch/sparc/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/sparc/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -670,8 +670,11 @@ asmlinkage int sparc_execve(struct pt_re (char __user * __user *)regs->u_regs[base + UREG_I2], regs); putname(filename); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } out: return error; } diff -puN arch/um/kernel/exec_kern.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/um/kernel/exec_kern.c --- 25/arch/um/kernel/exec_kern.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/um/kernel/exec_kern.c Thu Sep 30 15:35:49 2004 @@ -43,7 +43,9 @@ static int execve1(char *file, char **ar #endif error = do_execve(file, argv, env, ¤t->thread.regs); if (error == 0){ + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); set_cmdline(current_cmd()); } return(error); diff -puN arch/x86_64/ia32/sys_ia32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/x86_64/ia32/sys_ia32.c --- 25/arch/x86_64/ia32/sys_ia32.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/x86_64/ia32/sys_ia32.c Thu Sep 30 15:35:49 2004 @@ -1135,8 +1135,11 @@ asmlinkage long sys32_execve(char __user if (IS_ERR(filename)) return error; error = compat_do_execve(filename, argv, envp, regs); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); return error; } diff -puN arch/x86_64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 arch/x86_64/kernel/process.c --- 25/arch/x86_64/kernel/process.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/arch/x86_64/kernel/process.c Thu Sep 30 15:35:49 2004 @@ -542,8 +542,11 @@ long sys_execve(char __user *name, char if (IS_ERR(filename)) return error; error = do_execve(filename, argv, envp, ®s); - if (error == 0) + if (error == 0) { + task_lock(current); current->ptrace &= ~PT_DTRACE; + task_unlock(current); + } putname(filename); return error; } diff -puN include/linux/ptrace.h~fix-ptrace_attach-race-with-real-parents-wait-calls-2 include/linux/ptrace.h --- 25/include/linux/ptrace.h~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/include/linux/ptrace.h Thu Sep 30 15:35:49 2004 @@ -63,6 +63,7 @@ #define PT_TRACE_EXEC 0x00000080 #define PT_TRACE_VFORK_DONE 0x00000100 #define PT_TRACE_EXIT 0x00000200 +#define PT_ATTACHED 0x00000400 /* parent != real_parent */ #define PT_TRACE_MASK 0x000003f4 diff -puN kernel/exit.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 kernel/exit.c --- 25/kernel/exit.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/kernel/exit.c Thu Sep 30 15:35:49 2004 @@ -1280,6 +1280,22 @@ static int wait_task_continued(task_t *p } +static inline int my_ptrace_child(struct task_struct *p) +{ + if (!(p->ptrace & PT_PTRACED)) + return 0; + if (!(p->ptrace & PT_ATTACHED)) + return 1; + /* + * This child was PTRACE_ATTACH'd. We should be seeing it only if + * we are the attacher. If we are the real parent, this is a race + * inside ptrace_attach. It is waiting for the tasklist_lock, + * which we have to switch the parent links, but has already set + * the flags in p->ptrace. + */ + return (p->parent != p->real_parent); +} + static long do_wait(pid_t pid, int options, struct siginfo __user *infop, int __user *stat_addr, struct rusage __user *ru) { @@ -1308,12 +1324,12 @@ repeat: switch (p->state) { case TASK_TRACED: - if (!(p->ptrace & PT_PTRACED)) + if (!my_ptrace_child(p)) continue; /*FALLTHROUGH*/ case TASK_STOPPED: if (!(options & WUNTRACED) && - !(p->ptrace & PT_PTRACED)) + !my_ptrace_child(p)) continue; retval = wait_task_stopped(p, ret == 2, (options & WNOWAIT), diff -puN kernel/ptrace.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 kernel/ptrace.c --- 25/kernel/ptrace.c~fix-ptrace_attach-race-with-real-parents-wait-calls-2 Thu Sep 30 15:35:49 2004 +++ 25-akpm/kernel/ptrace.c Thu Sep 30 15:35:49 2004 @@ -82,7 +82,8 @@ int ptrace_check_attach(struct task_stru */ read_lock(&tasklist_lock); if ((child->ptrace & PT_PTRACED) && child->parent == current && - child->signal != NULL) { + (!(child->ptrace & PT_ATTACHED) || child->real_parent != current) + && child->signal != NULL) { ret = 0; spin_lock_irq(&child->sighand->siglock); if (child->state == TASK_STOPPED) { @@ -131,7 +132,7 @@ int ptrace_attach(struct task_struct *ta goto bad; /* Go */ - task->ptrace |= PT_PTRACED; + task->ptrace |= PT_PTRACED | PT_ATTACHED; if (capable(CAP_SYS_PTRACE)) task->ptrace |= PT_PTRACE_CAP; task_unlock(task); _