From e03d13e985d48ac4885382c9e3b1510c78bd047f Mon Sep 17 00:00:00 2001 From: Roland McGrath Date: Wed, 19 Oct 2005 22:21:23 -0700 Subject: [PATCH] [PATCH] Fix cpu timers exit deadlock and races Oleg Nesterov reported an SMP deadlock. If there is a running timer tracking a different process's CPU time clock when the process owning the timer exits, we deadlock on tasklist_lock in posix_cpu_timer_del via exit_itimers. That code was using tasklist_lock to check for a race with __exit_signal being called on the timer-target task and clearing its ->signal. However, there is actually no such race. __exit_signal will have called posix_cpu_timers_exit and posix_cpu_timers_exit_group before it does that. Those will clear those k_itimer's association with the dying task, so posix_cpu_timer_del will return early and never reach the code in question. In addition, posix_cpu_timer_del called from exit_itimers during execve or directly from timer_delete in the process owning the timer can race with an exiting timer-target task to cause a double put on timer-target task struct. Make sure we always access cpu_timers lists with sighand lock held. Signed-off-by: Roland McGrath Signed-off-by: Chris Wright Signed-off-by: Linus Torvalds --- kernel/posix-cpu-timers.c | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/kernel/posix-cpu-timers.c b/kernel/posix-cpu-timers.c index 7a51a5597c..b3f3edc475 100644 --- a/kernel/posix-cpu-timers.c +++ b/kernel/posix-cpu-timers.c @@ -387,25 +387,19 @@ int posix_cpu_timer_del(struct k_itimer *timer) if (unlikely(p == NULL)) return 0; + spin_lock(&p->sighand->siglock); if (!list_empty(&timer->it.cpu.entry)) { - read_lock(&tasklist_lock); - if (unlikely(p->signal == NULL)) { - /* - * We raced with the reaping of the task. - * The deletion should have cleared us off the list. - */ - BUG_ON(!list_empty(&timer->it.cpu.entry)); - } else { - /* - * Take us off the task's timer list. - */ - spin_lock(&p->sighand->siglock); - list_del(&timer->it.cpu.entry); - spin_unlock(&p->sighand->siglock); - } - read_unlock(&tasklist_lock); + /* + * Take us off the task's timer list. We don't need to + * take tasklist_lock and check for the task being reaped. + * If it was reaped, it already called posix_cpu_timers_exit + * and posix_cpu_timers_exit_group to clear all the timers + * that pointed to it. + */ + list_del(&timer->it.cpu.entry); + put_task_struct(p); } - put_task_struct(p); + spin_unlock(&p->sighand->siglock); return 0; } -- 2.20.1