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_races2010-09-20
03:53:32.0 +0200
+++ kstub/kernel/ptrace-utrace.c2010-09-20 03:53:32.0 +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.
