2008-06-10 16:49:45

by Oleg Nesterov

[permalink] [raw]
Subject: [PATCH 2/2] ptrace: never sleep in TASK_TRACED if SIGKILL'ed

Change ptrace_stop() to check if the task was killed unconditionally and
simplify the "if (killed)" case - we can just return. Note that SIGKILL
can be already dequeued if ptrace_stop() is called from do_exit(), that
is why we use signal_group_exit().

Note: this adds another user visible change. If the tracee was killed
because another thread execs, we are not going to sleep anyway. If this
change is not desirable, we can check SIGNAL_GROUP_EXIT instead.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 26-rc2/kernel/signal.c~2_PT_STOP_KILL 2008-06-08 16:15:25.000000000 +0400
+++ 26-rc2/kernel/signal.c 2008-06-10 20:06:24.000000000 +0400
@@ -1493,17 +1493,6 @@ static inline int may_ptrace_stop(void)
}

/*
- * Return nonzero if there is a SIGKILL that should be waking us up.
- * Called with the siglock held.
- */
-static int sigkill_pending(struct task_struct *tsk)
-{
- return ((sigismember(&tsk->pending.signal, SIGKILL) ||
- sigismember(&tsk->signal->shared_pending.signal, SIGKILL)) &&
- !unlikely(sigismember(&tsk->blocked, SIGKILL)));
-}
-
-/*
* This must be called with current->sighand->siglock held.
*
* This should be the path for all ptrace stops.
@@ -1516,8 +1505,6 @@ static int sigkill_pending(struct task_s
*/
static void ptrace_stop(int exit_code, int clear_code, siginfo_t *info)
{
- int killed = 0;
-
if (arch_ptrace_stop_needed(exit_code, info)) {
/*
* The arch code has something special to do before a
@@ -1526,16 +1513,15 @@ static void ptrace_stop(int exit_code, i
* calling arch_ptrace_stop, so we must release it now.
* To preserve proper semantics, we must do this before
* any signal bookkeeping like checking group_stop_count.
- * Meanwhile, a SIGKILL could come in before we retake the
- * siglock. That must prevent us from sleeping in TASK_TRACED.
- * So after regaining the lock, we must check for SIGKILL.
*/
spin_unlock_irq(&current->sighand->siglock);
arch_ptrace_stop(exit_code, info);
spin_lock_irq(&current->sighand->siglock);
- killed = sigkill_pending(current);
}

+ if (unlikely(signal_group_exit(current->signal)))
+ return;
+
/*
* If there is a group stop in progress,
* we must participate in the bookkeeping.
@@ -1550,7 +1536,7 @@ static void ptrace_stop(int exit_code, i
__set_current_state(TASK_TRACED);
spin_unlock_irq(&current->sighand->siglock);
read_lock(&tasklist_lock);
- if (!unlikely(killed) && may_ptrace_stop()) {
+ if (may_ptrace_stop()) {
do_notify_parent_cldstop(current, CLD_TRAPPED);
read_unlock(&tasklist_lock);
schedule();