[PATCH] fix PTRACE_ATTACH race with real parent's wait calls

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 <roland@redhat.com>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

2 files changed, 21 insertions, 4 deletions

diff --git a/kernel/exit.c b/kernel/exit.c
index 426d3ae722ba..ca9a9e21c444 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1280,6 +1280,22 @@ static int wait_task_continued(task_t *p, int noreap,
 }
 
 
+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 --git a/kernel/ptrace.c b/kernel/ptrace.c
index b14b4a467729..09ba057222c3 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -82,7 +82,8 @@ int ptrace_check_attach(struct task_struct *child, int kill)
 	 */
 	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 *task)
 		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);