linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <tj@kernel.org>
To: oleg@redhat.com, jan.kratochvil@redhat.com, vda.linux@googlemail.com
Cc: linux-kernel@vger.kernel.org, torvalds@linux-foundation.org,
	akpm@linux-foundation.org, indan@nul.nu, roland@hack.frob.com,
	Tejun Heo <tj@kernel.org>
Subject: [PATCH 12/20] ptrace: Always put ptracee into appropriate execution state
Date: Wed, 23 Mar 2011 11:05:58 +0100	[thread overview]
Message-ID: <1300874766-12941-13-git-send-email-tj@kernel.org> (raw)
In-Reply-To: <1300874766-12941-1-git-send-email-tj@kernel.org>

Currently, __ptrace_unlink() wakes up the tracee iff it's in
TASK_TRACED.  For unlinking from PTRACE_DETACH, this is correct as the
tracee is guaranteed to be in TASK_TRACED or dead; however, unlinking
also happens when the ptracer exits and in this case the ptracee can
be in any state and ptrace might be left running even if the group it
belongs to is stopped.

This patch updates __ptrace_unlink() such that GROUP_STOP_PENDING is
reinstated regardless of the ptracee's current state as long as it's
alive and makes sure that signal_wake_up() is called if execution
state transition is necessary.

Test case follows.

  #include <unistd.h>
  #include <time.h>
  #include <sys/types.h>
  #include <sys/ptrace.h>
  #include <sys/wait.h>

  static const struct timespec ts1s = { .tv_sec = 1 };

  int main(void)
  {
	  pid_t tracee;
	  siginfo_t si;

	  tracee = fork();
	  if (tracee == 0) {
		  while (1) {
			  nanosleep(&ts1s, NULL);
			  write(1, ".", 1);
		  }
	  }

	  ptrace(PTRACE_ATTACH, tracee, NULL, NULL);
	  waitid(P_PID, tracee, &si, WSTOPPED);
	  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
	  waitid(P_PID, tracee, &si, WSTOPPED);
	  ptrace(PTRACE_CONT, tracee, NULL, (void *)(long)si.si_status);
	  write(1, "exiting", 7);
	  return 0;
  }

Before the patch, after the parent process exits, the child is left
running and prints out "." every second.

  exiting..... (continues)

After the patch, the group stop initiated by the implied SIGSTOP from
PTRACE_ATTACH is re-established when the parent exits.

  exiting

Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
---
 kernel/ptrace.c |   59 ++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index e609843..4348586 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -41,7 +41,26 @@ void __ptrace_link(struct task_struct *child, struct task_struct *new_parent)
  * __ptrace_unlink - unlink ptracee and restore its execution state
  * @child: ptracee to be unlinked
  *
- * Remove @child from the ptrace list, move it back to the original parent.
+ * Remove @child from the ptrace list, move it back to the original parent,
+ * and restore the execution state so that it conforms to the group stop
+ * state.
+ *
+ * Unlinking can happen via two paths - explicit PTRACE_DETACH or ptracer
+ * exiting.  For PTRACE_DETACH, unless the ptracee has been killed between
+ * ptrace_check_attach() and here, it's guaranteed to be in TASK_TRACED.
+ * If the ptracer is exiting, the ptracee can be in any state.
+ *
+ * After detach, the ptracee should be in a state which conforms to the
+ * group stop.  If the group is stopped or in the process of stopping, the
+ * ptracee should be put into TASK_STOPPED; otherwise, it should be woken
+ * up from TASK_TRACED.
+ *
+ * If the ptracee is in TASK_TRACED and needs to be moved to TASK_STOPPED,
+ * it goes through TRACED -> RUNNING -> STOPPED transition which is similar
+ * to but in the opposite direction of what happens while attaching to a
+ * stopped task.  However, in this direction, the intermediate RUNNING
+ * state is not hidden even from the current ptracer and if it immediately
+ * re-attaches and performs a WNOHANG wait(2), it may fail.
  *
  * CONTEXT:
  * write_lock_irq(tasklist_lock)
@@ -55,25 +74,25 @@ void __ptrace_unlink(struct task_struct *child)
 	list_del_init(&child->ptrace_entry);
 
 	spin_lock(&child->sighand->siglock);
-	if (task_is_traced(child)) {
-		/*
-		 * If group stop is completed or in progress, it should
-		 * participate in the group stop.  Set GROUP_STOP_PENDING
-		 * before kicking it.
-		 *
-		 * This involves TRACED -> RUNNING -> STOPPED transition
-		 * which is similar to but in the opposite direction of
-		 * what happens while attaching to a stopped task.
-		 * However, in this direction, the intermediate RUNNING
-		 * state is not hidden even from the current ptracer and if
-		 * it immediately re-attaches and performs a WNOHANG
-		 * wait(2), it may fail.
-		 */
-		if (child->signal->flags & SIGNAL_STOP_STOPPED ||
-		    child->signal->group_stop_count)
-			child->group_stop |= GROUP_STOP_PENDING;
-		signal_wake_up(child, 1);
-	}
+
+	/*
+	 * Reinstate GROUP_STOP_PENDING if group stop is in effect and
+	 * @child isn't dead.
+	 */
+	if (!(child->flags & PF_EXITING) &&
+	    (child->signal->flags & SIGNAL_STOP_STOPPED ||
+	     child->signal->group_stop_count))
+		child->group_stop |= GROUP_STOP_PENDING;
+
+	/*
+	 * If transition to TASK_STOPPED is pending or in TASK_TRACED, kick
+	 * @child in the butt.  Note that @resume should be used iff @child
+	 * is in TASK_TRACED; otherwise, we might unduly disrupt
+	 * TASK_KILLABLE sleeps.
+	 */
+	if (child->group_stop & GROUP_STOP_PENDING || task_is_traced(child))
+		signal_wake_up(child, task_is_traced(child));
+
 	spin_unlock(&child->sighand->siglock);
 }
 
-- 
1.7.1


  parent reply	other threads:[~2011-03-23 10:06 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-23 10:05 [PATCHSET] ptrace,signal: Improve ptrace and job control interaction Tejun Heo
2011-03-23 10:05 ` [PATCH 01/20] signal: Fix SIGCONT notification code Tejun Heo
2011-03-23 10:05 ` [PATCH 02/20] ptrace: Remove the extra wake_up_state() from ptrace_detach() Tejun Heo
2011-03-23 10:05 ` [PATCH 03/20] signal: Remove superflous try_to_freeze() loop in do_signal_stop() Tejun Heo
2011-03-23 10:05 ` [PATCH 04/20] ptrace: Kill tracehook_notify_jctl() Tejun Heo
2011-03-23 10:05 ` [PATCH 05/20] ptrace: Add @why to ptrace_stop() Tejun Heo
2011-03-23 10:05 ` [PATCH 06/20] signal: Fix premature completion of group stop when interfered by ptrace Tejun Heo
2011-03-23 10:05 ` [PATCH 07/20] signal: Use GROUP_STOP_PENDING to stop once for a single group stop Tejun Heo
2011-03-23 10:05 ` [PATCH 08/20] ptrace: Participate in group stop from ptrace_stop() iff the task is trapping for " Tejun Heo
2011-03-23 10:05 ` [PATCH 09/20] ptrace: Make do_signal_stop() use ptrace_stop() if the task is being ptraced Tejun Heo
2011-03-23 10:05 ` [PATCH 10/20] ptrace: Clean transitions between TASK_STOPPED and TRACED Tejun Heo
2011-03-23 10:05 ` [PATCH 11/20] ptrace: Collapse ptrace_untrace() into __ptrace_unlink() Tejun Heo
2011-03-23 10:05 ` Tejun Heo [this message]
2011-03-23 10:05 ` [PATCH 13/20] job control: Don't set group_stop exit_code if re-entering job control stop Tejun Heo
2011-03-23 10:06 ` [PATCH 14/20] job control: Small reorganization of wait_consider_task() Tejun Heo
2011-03-23 10:06 ` [PATCH 15/20] job control: Fix ptracer wait(2) hang and explain notask_error clearing Tejun Heo
2011-03-23 10:06 ` [PATCH 16/20] job control: Allow access to job control events through ptracees Tejun Heo
2011-03-23 10:06 ` [PATCH 17/20] job control: Add @for_ptrace to do_notify_parent_cldstop() Tejun Heo
2011-03-23 10:06 ` [PATCH 18/20] job control: Job control stop notifications should always go to the real parent Tejun Heo
2011-03-23 10:06 ` [PATCH 19/20] job control: Notify the real parent of job control events regardless of ptrace Tejun Heo
2011-03-23 10:06 ` [PATCH 20/20] job control: Don't send duplicate job control stop notification while ptraced Tejun Heo
2011-03-23 18:38 ` [PATCHSET] ptrace,signal: Improve ptrace and job control interaction Oleg Nesterov
2011-03-25 14:26   ` Tejun Heo
2011-03-26 18:25     ` Oleg Nesterov
2011-03-28  8:58       ` Tejun Heo
2011-03-28 12:14         ` Oleg Nesterov
2011-03-28 15:21           ` Tejun Heo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1300874766-12941-13-git-send-email-tj@kernel.org \
    --to=tj@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=indan@nul.nu \
    --cc=jan.kratochvil@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=roland@hack.frob.com \
    --cc=torvalds@linux-foundation.org \
    --cc=vda.linux@googlemail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).