From: Oleg Nesterov <oleg@redhat.com> Date: Mon, 24 Nov 2008 18:22:57 +0100 Subject: [misc] fix check_dead_utrace vs do_wait() race Message-id: 20081124172257.GA19279@redhat.com O-Subject: [RHEL5 PATCH] BZ#466774: fix check_dead_utrace() vs do_wait() race Bugzilla: 466774 RH-Acked-by: Jerome Marchand <jmarchan@redhat.com> RH-Acked-by: Anton Arapov <aarapov@redhat.com> RH-Acked-by: Neil Horman <nhorman@redhat.com> See https://bugzilla.redhat.com/show_bug.cgi?id=466774 Hopefully addresses 428693,455025 as well. check_dead_utrace() plays with ->exit_state/exit_signal under read_lock(taslist_lock), we can race with do_wait() which finds this task in EXIT_ZOMBIE state and without UTRACE_ACTION_NOREAP, because check_dead_utrace() clears ->utrace_flags. Change the code to take tasklist_lock for writing. With this patch utrace->lock under tasklist_lock is forbidden. And the patch assumes nobody does so, or it is buggy. Signed-off-by: Oleg Nesterov <oleg@redhat.com> diff --git a/kernel/utrace.c b/kernel/utrace.c index 5bc6e58..2e36ac0 100644 --- a/kernel/utrace.c +++ b/kernel/utrace.c @@ -269,13 +269,13 @@ check_dead_utrace(struct task_struct *tsk, struct utrace *utrace, * 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. + * tasklist_lock excludes de_thread until we decide what + * to do. */ - read_lock(&tasklist_lock); + write_lock_irq(&tasklist_lock); if (tsk->exit_signal == -1) { /* Self-reaping thread. */ exit_state = xchg(&tsk->exit_state, EXIT_DEAD); - read_unlock(&tasklist_lock); + write_unlock_irq(&tasklist_lock); BUG_ON(exit_state != EXIT_ZOMBIE); exit_state = EXIT_DEAD; /* Reap it below. */ @@ -294,9 +294,9 @@ check_dead_utrace(struct task_struct *tsk, struct utrace *utrace, * the UTRACE_ACTION_NOREAP bit is cleared. It's * safe for that to do everything it does until its * release_task call starts tearing things down. - * Holding tasklist_lock for reading prevents - * release_task from proceeding until we've done - * everything we need to do. + * Holding tasklist_lock for writing prevents + * release_task/do_wait from proceeding until + * we've done everything we need to do. */ exit_state = EXIT_ZOMBIE; else @@ -306,7 +306,7 @@ check_dead_utrace(struct task_struct *tsk, struct utrace *utrace, * group leader in an exec by another thread, * which will call release_task itself. */ - read_unlock(&tasklist_lock); + write_unlock_irq(&tasklist_lock); } /* @@ -345,7 +345,7 @@ check_dead_utrace(struct task_struct *tsk, struct utrace *utrace, BUG_ON(exit_state != EXIT_ZOMBIE); exit_state = EXIT_DEAD; /* Reap it below. */ } - read_unlock(&tasklist_lock); /* See comment above. */ + write_unlock_irq(&tasklist_lock); /* See comment above. */ } if (exit_state == EXIT_DEAD) /*