Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754764Ab0KZKuQ (ORCPT ); Fri, 26 Nov 2010 05:50:16 -0500 Received: from hera.kernel.org ([140.211.167.34]:42299 "EHLO hera.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754740Ab0KZKuO (ORCPT ); Fri, 26 Nov 2010 05:50:14 -0500 From: Tejun Heo To: roland@redhat.com, oleg@redhat.com, linux-kernel@vger.kernel.org, torvalds@linux-foundation.org, akpm@linux-foundation.org, "rjw@sisk.plpavel"@ucw.cz Cc: Tejun Heo Subject: [PATCH 13/14] ptrace: make SIGCONT notification reliable against ptrace Date: Fri, 26 Nov 2010 11:49:28 +0100 Message-Id: <1290768569-16224-14-git-send-email-tj@kernel.org> X-Mailer: git-send-email 1.7.1 In-Reply-To: <1290768569-16224-1-git-send-email-tj@kernel.org> References: <1290768569-16224-1-git-send-email-tj@kernel.org> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.2.3 (hera.kernel.org [127.0.0.1]); Fri, 26 Nov 2010 10:49:50 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5871 Lines: 169 Currently, SIGCONT notifications which are pending on ptrace attach or occur while ptraced are reported to the tracer and never make it to the real parent. This patch adds a new signal flag SIGNAL_NOTIFY_CONT which is set when a task is woken up by SIGCONT and cleared once the event is notified to the parent. SIGNAL_CLD_MASK bits are no longer cleared after notification. Combined with clearing SIGNAL_CLD_MASK if !SIGNAL_NOTIFY_CONT on ptrace attach, these bits are set on ptrace detach iff the tracee owes a notification to the real parent. __ptrace_unlink() is updated to check these bits and reschedule SIGCONT notification if necessary. This combined with the previous changes makes ptrace attach/detach mostly transparent with respect to job control signal handling. The remaining problems are the extra unconditional wake_up_process() from ptrace_detach() and SIGSTOP generated by ptrace_attach() clearing pending SIGCONT. These will be dealt with future patches. Signed-off-by: Tejun Heo Cc: Oleg Nesterov Cc: Roland McGrath --- include/linux/sched.h | 1 + kernel/ptrace.c | 40 +++++++++++++++++++++++++++++++++++++++- kernel/signal.c | 14 +++++++++----- 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 3e40761..4b7f3ca 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -654,6 +654,7 @@ struct signal_struct { #define SIGNAL_UNKILLABLE 0x00000040 /* for init: ignore fatal signals */ #define SIGNAL_NOTIFY_STOP 0x00000100 /* notify parent of group stop */ +#define SIGNAL_NOTIFY_CONT 0x00000200 /* notify parent of continuation */ /* If true, all threads except ->group_exit_task have pending SIGKILL */ static inline int signal_group_exit(const struct signal_struct *sig) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 71141bf..a6c92ac 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -52,6 +52,7 @@ void __ptrace_link(struct task_struct *child, struct task_struct *new_parent) void __ptrace_unlink(struct task_struct *child) { struct signal_struct *sig = child->signal; + bool woken_up = false; BUG_ON(!child->ptrace); @@ -66,6 +67,33 @@ void __ptrace_unlink(struct task_struct *child) if (sig->flags & SIGNAL_STOP_STOPPED || sig->group_stop_count) child->group_stop |= GROUP_STOP_PENDING; signal_wake_up(child, 1); + woken_up = true; + } + + /* + * SIGNAL_CLD_MASK is cleared only on a stop signal or, if + * notification isn't pending, ptrace attach. If any bit is + * set, + * + * - SIGCONT notification was pending before attach or there + * was one or more SIGCONT notifications while tracing. + * + * - And, there hasn't been any stop signal since the last + * pending SIGCONT notification. + * + * Combined, it means that the tracee owes a SIGCONT + * notification to the real parent. + */ + if (sig->flags & SIGNAL_CLD_MASK) { + sig->flags |= SIGNAL_NOTIFY_CONT; + /* + * Force the tracee into signal delivery path so that + * the notification is delievered ASAP. This wakeup + * is unintrusive as SIGCONT delivery would have + * caused the same effect. + */ + if (!woken_up) + signal_wake_up(child, 0); } child->ptrace = 0; @@ -198,17 +226,27 @@ int ptrace_attach(struct task_struct *task) __ptrace_link(task, current); send_sig_info(SIGSTOP, SEND_SIG_FORCED, task); + spin_lock(&task->sighand->siglock); + /* * If the task is already STOPPED, set GROUP_STOP_PENDING and * kick it so that it transits to TRACED. This is safe as * both transitions in and out of STOPPED are protected by * siglock. */ - spin_lock(&task->sighand->siglock); if (task_is_stopped(task)) { task->group_stop |= GROUP_STOP_PENDING; signal_wake_up(task, 1); } + + /* + * Clear SIGNAL_CLD_MASK if NOTIFY_CONT is not set. This is + * used to preserve SIGCONT notification across ptrace + * attach/detach. Read the comment in __ptrace_unlink(). + */ + if (!(task->signal->flags & SIGNAL_NOTIFY_CONT)) + task->signal->flags &= ~SIGNAL_CLD_MASK; + spin_unlock(&task->sighand->siglock); retval = 0; diff --git a/kernel/signal.c b/kernel/signal.c index f2da456..735bac5 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -781,7 +781,8 @@ static int prepare_signal(int sig, struct task_struct *p, int from_ancestor_ns) * will take ->siglock, notice SIGNAL_CLD_MASK, and * notify its parent. See get_signal_to_deliver(). */ - signal->flags = why | SIGNAL_STOP_CONTINUED; + why |= SIGNAL_STOP_CONTINUED | SIGNAL_NOTIFY_CONT; + signal->flags = why; signal->group_stop_count = 0; signal->group_exit_code = 0; } else { @@ -1895,7 +1896,7 @@ relock: * we should notify the parent, prepare_signal(SIGCONT) encodes * the CLD_ si_code into SIGNAL_CLD_MASK bits. */ - if (unlikely(signal->flags & SIGNAL_CLD_MASK)) { + if (unlikely(signal->flags & SIGNAL_NOTIFY_CONT)) { int why; if (signal->flags & SIGNAL_CLD_CONTINUED) @@ -1903,7 +1904,7 @@ relock: else why = CLD_STOPPED; - signal->flags &= ~SIGNAL_CLD_MASK; + signal->flags &= ~SIGNAL_NOTIFY_CONT; why = tracehook_notify_jctl(why, CLD_CONTINUED); spin_unlock_irq(&sighand->siglock); @@ -1942,8 +1943,11 @@ relock: if (signr != SIGKILL) { signr = ptrace_signal(signr, info, regs, cookie); - if (!signr) - continue; + if (!signr) { + /* NOTIFY_CONT might have changed */ + spin_unlock_irq(&sighand->siglock); + goto relock; + } } ka = &sighand->action[signr-1]; -- 1.7.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/