Finally, the actual fix.
ptrace_detach_task(sig => -1) is very buggy. Somehow I completely forgot
that implicit detach can race with the running tracee. Depending on how
exactly it races with ptrace_report_signal() we can have the following
problems:
1) If the tracer exits right after ptrace(PTRACE_CONT, SIGXXX)
the tracee can miss this signal.
2) the tracee can report a signal and stop after it was already
detached.
Change ptrace_detach_task(sig => -1) to set ctx->resume = UTRACE_DETACH
before anything else, change ptrace_report_signal() to check ctx->resume
before reporting a signal. This fixes the 2nd problem.
To fix the 1st problem, change !explicit case to check ->siginfo != NULL
instead of relying on get_stop_event() which can't work with the running
tracee.
Reported-by: Glen Johnson <bugpr...@us.ibm.com>
Signed-off-by: Oleg Nesterov <o...@redhat.com>
---
kernel/ptrace-utrace.c | 41 ++++++++++++++++++++++++++++++++++-------
1 file changed, 34 insertions(+), 7 deletions(-)
--- kstub/kernel/ptrace-utrace.c~5_implicit_detach_races 2010-09-20
03:53:32.000000000 +0200
+++ kstub/kernel/ptrace-utrace.c 2010-09-20 03:53:32.000000000 +0200
@@ -262,7 +262,7 @@ static void ptrace_detach_task(struct ta
* If true, the caller is PTRACE_DETACH, otherwise
* the tracer detaches implicitly during exit.
*/
- bool voluntary = (sig >= 0);
+ bool explicit = (sig >= 0);
struct utrace_engine *engine = ptrace_lookup_engine(tracee);
enum utrace_resume_action action = UTRACE_DETACH;
struct ptrace_context *ctx;
@@ -272,24 +272,46 @@ static void ptrace_detach_task(struct ta
ctx = ptrace_context(engine);
- if (sig) {
+ if (!explicit) {
+ int err;
+ /*
+ * We are going to detach, the tracee can be running.
+ * Ensure ptrace_report_signal() won't report a signal.
+ */
+ ctx->resume = UTRACE_DETACH;
+ err = utrace_barrier_uninterruptible(tracee, engine);
+
+ if (!err && ctx->siginfo) {
+ /*
+ * The tracee has already reported a signal
+ * before utrace_barrier().
+ *
+ * Resume it like we do in PTRACE_EVENT_SIGNAL
+ * case below. The difference is that we can race
+ * with ptrace_report_signal() if the tracee is
+ * running but this doesn't matter. In any case
+ * UTRACE_SIGNAL_REPORT must be pending and it
+ * can return nothing but UTRACE_DETACH.
+ */
+ action = UTRACE_RESUME;
+ }
+
+ } else if (sig) {
switch (get_stop_event(ctx)) {
case PTRACE_EVENT_SYSCALL:
- if (voluntary)
- send_sig_info(sig, SEND_SIG_PRIV, tracee);
+ send_sig_info(sig, SEND_SIG_PRIV, tracee);
break;
case PTRACE_EVENT_SIGNAL:
- if (voluntary)
- ctx->signr = sig;
+ ctx->signr = sig;
ctx->resume = UTRACE_DETACH;
action = UTRACE_RESUME;
break;
}
}
- ptrace_wake_up(tracee, engine, action, voluntary);
+ ptrace_wake_up(tracee, engine, action, explicit);
if (action != UTRACE_DETACH)
ctx->options = PTRACE_O_DETACHED;
@@ -553,6 +575,11 @@ static u32 ptrace_report_signal(u32 acti
}
WARN_ON(ctx->siginfo);
+
+ /* Raced with the exiting tracer ? */
+ if (resume == UTRACE_DETACH)
+ return action;
+
ctx->siginfo = info;
/*
* ctx->siginfo points to the caller's stack.
