[PATCH 1/2] utrace: finish_resume_report: don't do user_xxx_step() if ptrace_wants_step()
finish_resume_report() should not enable/disable the stepping if
ptrace_wants_step() == T. If ptrace wants block_step while utrace
wants single_step we could promote the stepping, but I do not
think this really makes sense.
Unless the tracee is killed this can't race with ptrace, this is
called by the tracee itself. If it is killed we do not care.
Signed-off-by: Oleg Nesterov o...@redhat.com
---
include/linux/tracehook.h |8
kernel/utrace.c |9 ++---
2 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/include/linux/tracehook.h b/include/linux/tracehook.h
index 06edb52..5612d2d 100644
--- a/include/linux/tracehook.h
+++ b/include/linux/tracehook.h
@@ -121,8 +121,8 @@ static inline __must_check int
tracehook_report_syscall_entry(
return 0;
}
-#define ptrace_wants_step()\
- (current-ptrace (PT_SINGLE_STEP | PT_SINGLE_BLOCK))
+#define ptrace_wants_step(task)\
+ ((task)-ptrace (PT_SINGLE_STEP | PT_SINGLE_BLOCK))
/**
* tracehook_report_syscall_exit - task has just finished a system call
@@ -146,7 +146,7 @@ static inline void tracehook_report_syscall_exit(struct
pt_regs *regs, int step)
if (task_utrace_flags(current) UTRACE_EVENT(SYSCALL_EXIT))
utrace_report_syscall_exit(regs);
- if (step ptrace_wants_step()) {
+ if (step ptrace_wants_step(current)) {
siginfo_t info;
user_single_step_siginfo(current, regs, info);
force_sig_info(SIGTRAP, info, current);
@@ -439,7 +439,7 @@ static inline void tracehook_signal_handler(int sig,
siginfo_t *info,
{
if (task_utrace_flags(current))
utrace_signal_handler(current, stepping);
- if (stepping ptrace_wants_step())
+ if (stepping ptrace_wants_step(current))
ptrace_notify(SIGTRAP);
}
diff --git a/kernel/utrace.c b/kernel/utrace.c
index 508c13c..fbabb81 100644
--- a/kernel/utrace.c
+++ b/kernel/utrace.c
@@ -1828,7 +1828,8 @@ static void finish_resume_report(struct task_struct *task,
case UTRACE_BLOCKSTEP:
if (likely(arch_has_block_step())) {
- user_enable_block_step(task);
+ if (!ptrace_wants_step(task))
+ user_enable_block_step(task);
break;
}
@@ -1841,7 +1842,8 @@ static void finish_resume_report(struct task_struct *task,
case UTRACE_SINGLESTEP:
if (likely(arch_has_single_step())) {
- user_enable_single_step(task);
+ if (!ptrace_wants_step(task))
+ user_enable_single_step(task);
} else {
/*
* This means some callback is to blame for failing
@@ -1856,7 +1858,8 @@ static void finish_resume_report(struct task_struct *task,
case UTRACE_REPORT:
case UTRACE_RESUME:
default:
- user_disable_single_step(task);
+ if (!ptrace_wants_step(task))
+ user_disable_single_step(task);
break;
}
}
--
1.5.5.1
[PATCH 2/2] 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 o...@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
