Attachment 149546 Details for Bug 228816 – fix for utrace/ptrace leak and crash bugs with MT exec + utrace_attach spin failure mode

[patch] fix for utrace/ptrace leak and crash bugs with MT exec + utrace_attach spin failure mode

linux-2.6-utrace-fixes-el5.patch (text/plain), 7.10 KB, created by Roland McGrath on 2007-03-08 03:10:44 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Roland McGrath

Created: 2007-03-08 03:10:44 UTC

Size: 7.10 KB

patch

obsolete

>commit ee34145523ae534539986b5e9bb06005bbff3791
>Author: Roland McGrath <roland@redhat.com>
>Date:   Tue Mar 6 23:39:06 2007 -0800
>
>    Protect NOREAP handling against unstoppable de_thread reaping from MT exec.
>
>diff --git a/kernel/utrace.c b/kernel/utrace.c
>index cec2e0a..277852b 100644  
>--- a/kernel/utrace.c
>+++ b/kernel/utrace.c
>@@ -202,17 +202,23 @@ check_dead_utrace(struct task_struct *ts
> 	 */
> 	if (((tsk->utrace_flags &~ flags) & UTRACE_ACTION_NOREAP)
> 	    && tsk->exit_state) {
>-		BUG_ON(tsk->exit_state != EXIT_ZOMBIE);
> 		/*
> 		 * While holding the utrace lock, mark that it's been done.
> 		 * For self-reaping, we need to change tsk->exit_state
> 		 * before clearing tsk->utrace_flags, so that the real
>-		 * parent can't see it in EXIT_ZOMBIE momentarily and reap it.
>+		 * parent can't see it in EXIT_ZOMBIE momentarily and reap
>+		 * it.  If tsk was the group_leader, an exec by another
>+		 * thread can release_task it despite our NOREAP.  Holding
>+		 * tasklist_lock for reading excludes de_thread until we
>+		 * decide what to do.
> 		 */
>-		if (tsk->exit_signal == -1) {
>+		read_lock(&tasklist_lock);
>+		if (tsk->exit_signal == -1) { /* Self-reaping thread.  */
> 			exit_state = xchg(&tsk->exit_state, EXIT_DEAD);
>+			read_unlock(&tasklist_lock);
>+
> 			BUG_ON(exit_state != EXIT_ZOMBIE);
>-			exit_state = EXIT_DEAD;
>+			exit_state = EXIT_DEAD;	/* Reap it below.  */
> 
> 			/*
> 			 * Now that we've changed its state to DEAD,
>@@ -220,7 +226,7 @@ check_dead_utrace(struct task_struct *ts
> 			 * value without the UTRACE_ACTION_NOREAP bit set.
> 			 */
> 		}
>-		else if (thread_group_empty(tsk)) {
>+		else if (thread_group_empty(tsk)) /* Normal solo zombie.  */
> 			/*
> 			 * We need to prevent the real parent from reaping
> 			 * until after we've called do_notify_parent, below.
>@@ -233,32 +239,47 @@ check_dead_utrace(struct task_struct *ts
> 			 * everything we need to do.
> 			 */
> 			exit_state = EXIT_ZOMBIE;
>-			read_lock(&tasklist_lock);
>-		}
>+		else
>+			/*
>+			 * Delayed group leader, nothing to do yet.
>+			 * This is also the situation with the old
>+			 * group leader in an exec by another thread,
>+			 * which will call release_task itself.
>+			 */
>+			read_unlock(&tasklist_lock);
>+
> 	}
> 
> 	tsk->utrace_flags = flags;
> 	if (flags)
> 		utrace_unlock(utrace);
>-	else {
>+	else
> 		rcu_utrace_free(utrace);
>-		utrace = NULL;
>-	}
> 
> 	/*
> 	 * Now we're finished updating the utrace state.
> 	 * Do a pending self-reaping or parent notification.
> 	 */
>+	if (exit_state == EXIT_ZOMBIE) {
>+		do_notify_parent(tsk, tsk->exit_signal);
>+
>+		/*
>+		 * If SIGCHLD was ignored, that set tsk->exit_signal = -1
>+		 * to tell us to reap it immediately.
>+		 */
>+		if (tsk->exit_signal == -1) {
>+			exit_state = xchg(&tsk->exit_state, EXIT_DEAD);
>+			BUG_ON(exit_state != EXIT_ZOMBIE);
>+			exit_state = EXIT_DEAD;	/* Reap it below.  */
>+		}
>+		read_unlock(&tasklist_lock); /* See comment above.  */
>+	}
> 	if (exit_state == EXIT_DEAD)
> 		/*
> 		 * Note this can wind up in utrace_reap and do more callbacks.
> 		 * Our callers must be in places where that is OK.
> 		 */
> 		release_task(tsk);
>-	else if (exit_state == EXIT_ZOMBIE) {
>-		do_notify_parent(tsk, tsk->exit_signal);
>-		read_unlock(&tasklist_lock); /* See comment above.  */
>-	}
> }
> 
> 
>
>commit 6dad5136660f1d58531ea235c0261e8c10dc766d
>Author: Roland McGrath <roland@redhat.com>
>Date:   Tue Mar 6 23:36:51 2007 -0800
>
>    Protect ptrace against unstoppable exec de_thread reaping.
>
>diff --git a/kernel/ptrace.c b/kernel/ptrace.c
>index d0cbf66..809a4fb 100644  
>--- a/kernel/ptrace.c
>+++ b/kernel/ptrace.c
>@@ -161,10 +161,19 @@ ptrace_update(struct task_struct *target
> 			state->u.live.u.siginfo = NULL;
> 
> 		if (target->state == TASK_STOPPED) {
>-			spin_lock_irq(&target->sighand->siglock);
>-			if (target->state == TASK_STOPPED)
>-				target->signal->flags &= ~SIGNAL_STOP_STOPPED;
>-			spin_unlock_irq(&target->sighand->siglock);
>+			/*
>+			 * We have to double-check for naughty de_thread
>+			 * reaping despite NOREAP, before we can get siglock.
>+			 */
>+			read_lock(&tasklist_lock);
>+			if (!target->exit_state) {
>+				spin_lock_irq(&target->sighand->siglock);
>+				if (target->state == TASK_STOPPED)
>+					target->signal->flags &=
>+						~SIGNAL_STOP_STOPPED;
>+				spin_unlock_irq(&target->sighand->siglock);
>+			}
>+			read_unlock(&tasklist_lock);
> 		}
> 	}
> 
>@@ -290,13 +299,23 @@ static int ptrace_attach(struct task_str
> 	if (retval)
> 		(void) utrace_detach(task, engine);
> 	else {
>-		int stopped;
>-
>-		force_sig_specific(SIGSTOP, task);
>+		int stopped = 0;
> 
>-		spin_lock_irq(&task->sighand->siglock);
>-		stopped = (task->state == TASK_STOPPED);
>-		spin_unlock_irq(&task->sighand->siglock);
>+		/*
>+		 * We must double-check that task has not just died and
>+		 * been reaped (after ptrace_update succeeded).
>+		 * This happens when exec (de_thread) ignores NOREAP.
>+		 * We cannot call into the signal code if it's dead.
>+		 */
>+		read_lock(&tasklist_lock);
>+		if (likely(!task->exit_state)) {
>+			force_sig_specific(SIGSTOP, task);
>+
>+			spin_lock_irq(&task->sighand->siglock);
>+			stopped = (task->state == TASK_STOPPED);
>+			spin_unlock_irq(&task->sighand->siglock);
>+		}
>+		read_unlock(&tasklist_lock);
> 
> 		if (stopped) {
> 			/*
>
>commit 5278b661c82df463c07f1de9b4f442d828502d92
>Author: Roland McGrath <roland@redhat.com>
>Date:   Tue Mar 6 23:42:13 2007 -0800
>
>    Fix utrace_engine_cache leak in utrace_attach race restart case.
>
>diff --git a/kernel/utrace.c b/kernel/utrace.c
>index 277852b..32bc89d 100644  
>--- a/kernel/utrace.c
>+++ b/kernel/utrace.c
>@@ -367,12 +367,12 @@ restart:
> 
> 	first:
> 		utrace = utrace_first_engine(target, engine);
>-		if (IS_ERR(utrace)) {
>+		if (IS_ERR(utrace) || unlikely(utrace == NULL)) {
> 			kmem_cache_free(utrace_engine_cachep, engine);
>+			if (unlikely(utrace == NULL)) /* Race condition.  */
>+				goto restart;
> 			return ERR_PTR(PTR_ERR(utrace));
> 		}
>-		if (unlikely(utrace == NULL)) /* Race condition.  */
>-			goto restart;
> 	}
> 	else if (unlikely(target->exit_state == EXIT_DEAD)) {
> 		/*
>
>commit 49132701e0a7e48690523009eef6430d7079fde5
>Author: Roland McGrath <roland@redhat.com>
>Date:   Wed Mar 7 19:01:23 2007 -0800
>
>    Fix utrace_attach race with reaping that leads to spinning.
>
>diff --git a/kernel/utrace.c b/kernel/utrace.c
>index 32bc89d..d55dca1 100644  
>--- a/kernel/utrace.c
>+++ b/kernel/utrace.c
>@@ -353,12 +353,19 @@ restart:
> 	rcu_read_lock();
> 	utrace = rcu_dereference(target->utrace);
> 	smp_rmb();
>+	if (unlikely(target->exit_state == EXIT_DEAD)) {
>+		/*
>+		 * The target has already been reaped.
>+		 * Check this first; a race with reaping may lead to restart.
>+		 */
>+		rcu_read_unlock();
>+		return ERR_PTR(-ESRCH);
>+	}
> 	if (utrace == NULL) {
> 		rcu_read_unlock();
> 
>-		if (!(flags & UTRACE_ATTACH_CREATE)) {
>+		if (!(flags & UTRACE_ATTACH_CREATE))
> 			return ERR_PTR(-ENOENT);
>-		}
> 
> 		engine = kmem_cache_alloc(utrace_engine_cachep, GFP_KERNEL);
> 		if (unlikely(engine == NULL))
>@@ -374,13 +381,6 @@ restart:
> 			return ERR_PTR(PTR_ERR(utrace));
> 		}
> 	}
>-	else if (unlikely(target->exit_state == EXIT_DEAD)) {
>-		/*
>-		 * The target has already been reaped.
>-		 */
>-		rcu_read_unlock();
>-		return ERR_PTR(-ESRCH);
>-	}
> 	else {
> 		if (!(flags & UTRACE_ATTACH_CREATE)) {
> 			engine = matching_engine(utrace, flags, ops, data);

Actions: View | Diff

Attachments on bug 228816: 149446 | 149481 | 149546