[PATCH 1/1] do_exit(): Solve possibility of BUG() due to race with try_to_wake_up()

From: Kautuk Consul
Date: Mon Aug 25 2014 - 06:54:52 EST

I encountered a BUG() scenario within do_exit() on an ARM system.

The problem is due to a race scenario between do_exit() and try_to_wake_up()
on different CPUs due to usage of sleeping primitives such as __down_common
and wait_for_common.

Race Scenario

Let us assume there are 2 CPUs A and B execute code in the following order:
1) CPU A was running in user-mode and enters kernel mode via some
syscall/exception handler.
2) CPU A sets the current task(t) state to TASK_INTERRUPTIBLE via __down_common
or wait_for_common.
3) CPU A checks for signal_pending() and returns due to TIF_SIGPENDING
being set in t's threadinfo due to a previous signal(say SIGKILL) being
received on this task t.
4) CPU A returns returns back to the assembly trap handler and calls
do_work_pending() -> do_signal() -> get_signal() -> do_group_exit()
-> do_exit()
CPU A has not yet executed the following line of code before the final
call to schedule:
/* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD;
5) CPU B tries to send a signal to task t (currently executing on CPU A)
and thus enters: signal_wake_up_state() -> wake_up_state() ->
6) CPU B executes all code in try_to_wake_up() till the call to
ttwu_queue -> ttwu_do_activate -> ttwu_do_wakeup().
CPU B has still not executed the following code in ttwu_do_wakeup():
p->state = TASK_RUNNING;
7) CPU A executes the following line of code:
/* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD;
8) CPU B executes the following code in ttwu_do_wakeup():
p->state = TASK_RUNNING;
9) CPU A continues to the call to do_exit() -> schedule().
Since the tsk->state is TASK_RUNNING, the call to schedule() returns and
do_exit() -> BUG() is hit on CPU A.

Alternate Solution

An alternate solution would be to simply set the current task state to
TASK_RUNNING in __down_common(), wait_for_common() and all other interruptible
sleeping primitives in their if(signal_pending/signal_pending_state) conditional

But this change seems to me to be more logical because:
i) This will involve lesser changes to the kernel core code.
ii) Any further sleeping primitives in the kernel also need not suffer from
this kind of race scenario.

Signed-off-by: Kautuk Consul <consul.kautuk@xxxxxxxxx>
kernel/exit.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 32c58f7..69a8231 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -824,14 +824,16 @@ void do_exit(long code)
* (or hypervisor of virtual machine switches to other guest)
* As a result, we may become TASK_RUNNING after becoming TASK_DEAD
- * To avoid it, we have to wait for releasing tsk->pi_lock which
- * is held by try_to_wake_up()
+ * To solve this, we have to compete for tsk->pi_lock which is held by
+ * try_to_wake_up().
- smp_mb();
- raw_spin_unlock_wait(&tsk->pi_lock);
+ raw_spin_lock(&tsk->pi_lock);

/* causes final put_task_struct in finish_task_switch(). */
tsk->state = TASK_DEAD;
+ raw_spin_unlock(&tsk->pi_lock);
tsk->flags |= PF_NOFREEZE; /* tell freezer to ignore us */

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/