From: Roland McGrath <roland@redhat.com> Subject: [RHEL5 day-0 patch] CVE-2007-0771 RHBZ#228816 / RHBZ#229886 Date: Thu, 8 Mar 2007 14:17:05 -0800 (PST) Bugzilla: 228816 Message-Id: <20070308221705.45BB8180063@magilla.sf.frob.com> Changelog: [utrace] exploit and unkillable cpu fixes This is a series of four patches (can be appended to linux-2.6-utrace.patch). These fixes address an exploitable crash scenario and unkillable CPU-chewing process scenario DoS in utrace, brought on by the peculiar things done by non-leader exec in a multithreaded process. commit ee34145523ae534539986b5e9bb06005bbff3791 Author: Roland McGrath <roland@redhat.com> 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> 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> 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> 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);