[PATCH 2/2] ptrace: shift user_*_step() from ptrace_resume() to ptrace_stop()

Oleg Nesterov oleg at redhat.com
Fri Jul 1 20:19:32 UTC 2011


1. ptrace_resume() plays with the stopped task which can be also
   task_is_utraced(). In this case user_enable_xxx_step() can race
   with utrace_reset()->user_disable_single_step().

   We could change utrace_reset() to check ptrace_wants_step() and
   add the task_utrace_lock + unlock barrier after ptrace_resume()
   sets PT_SINGLE_STEP.

   But it is better to reassign enable/desable from the tracer to
   the tracee, it can check its PT_SINGLE_ bits after wakeup. This
   also makes sense because enable_step(task) can be faster if
   task == current.

2. ptrace can do user_disable_single_step() while utrace needs the
   stepping.

   Set TIF_NOTIFY_RESUME after user_disable_single_step() to ensure
   the tracee can't return to the user-space without utrace_resume().
   Any correct engine which wants the stepping should reassert it.

Signed-off-by: Oleg Nesterov <oleg at redhat.com>
---
 kernel/ptrace.c |    4 ----
 kernel/signal.c |   12 ++++++++++++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 44908d0..d37b30d 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -580,14 +580,10 @@ static int ptrace_resume(struct task_struct *child, long request,
 		if (unlikely(!arch_has_block_step()))
 			return -EIO;
 		child->ptrace |= PT_SINGLE_BLOCK;
-		user_enable_block_step(child);
 	} else if (is_singlestep(request) || is_sysemu_singlestep(request)) {
 		if (unlikely(!arch_has_single_step()))
 			return -EIO;
 		child->ptrace |= PT_SINGLE_STEP;
-		user_enable_single_step(child);
-	} else {
-		user_disable_single_step(child);
 	}
 
 	child->exit_code = data;
diff --git a/kernel/signal.c b/kernel/signal.c
index d0e0c67..bc40bd7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1840,6 +1840,18 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
 	}
 
 	tracehook_finish_stop();
+
+	if (current->ptrace & PT_SINGLE_BLOCK)
+		user_enable_block_step(current);
+	else if (current->ptrace & PT_SINGLE_STEP)
+		user_enable_single_step(current);
+	else {
+		user_disable_single_step(current);
+		/* if utrace needs the stepping it should reassert */
+		if (task_utrace_flags(current))
+			set_thread_flag(TIF_NOTIFY_RESUME);
+	}
+
 	/*
 	 * While in TASK_TRACED, we were considered "frozen enough".
 	 * Now that we woke up, it's crucial if we're supposed to be
-- 
1.5.5.1




More information about the kernel mailing list