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

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(a)redhat.com&gt; --- kernel/ptrace.c | 4 ---- kernel/signal.c | 11 +++++++++++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/kernel/ptrace.c b/kernel/ptrace.c index d250a71..d1ef124 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -635,14 +635,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 3c783d3..43a3e77 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1920,6 +1920,17 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info) utrace_end_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