On 09/22, Oleg Nesterov wrote:
>
> On 09/21, Roland McGrath wrote:
> >
> > > I was worried about the case when the TIF_SINGLESTEP tracee detaches
> > > itself and escapes finish_resume_report()->user_disable_single_step(),
> > > say, utrace_report_exec(). But probably we can ignore this.
> >
> > No, I think that is indeed a problem. If no engine is still attached
> > whose last callback returned UTRACE_SINGLESTEP, we should never return
> > to user mode with single-step enabled.
>
> OK, so what should we do for now?
>
> Without the multitracing utrace_resume()->user_disable_single_step()
> fixes the problem. Otherwise we probably need ENGINE_WANTS_SINGLESTEP.
As expected, the patch below equally fixes the problem and passes
ptrace-tests.
Which one do you prefer?
Note that, since we are going to change UTRACE_RESUME to remove
ENGINE_STOP from utrace_flags, we can probably unify DETACH/RESUME
case UTRACE_DETACH:
mark_engine_detached(engine);
reset = reset || utrace_do_stop(target, utrace);
if (!reset) {
/*
* As in utrace_set_events(), this barrier ensures
* that our engine->flags changes have hit before we
* examine utrace->reporting, pairing with the barrier
* in start_callback(). If @target has not yet hit
* finish_callback() to clear utrace->reporting, we
* might be in the middle of a callback to @engine.
*/
smp_mb();
if (utrace->reporting == engine)
ret = -EINPROGRESS;
}
/* fall */
case UTRACE_RESUME:
clear_engine_wants_stop(engine);
target->utrace_flags &= ~ENGINE_STOP;
/*
* This and all other cases imply resuming if stopped.
* There might not be another report before it just
* resumes, so make sure single-step is not left set.
*/
if (likely(reset))
user_disable_single_step(target);
break;
--- kstub/kernel/utrace.c~10_utrace_reset_should_clear_ss 2010-09-20
03:55:12.000000000 +0200
+++ kstub/kernel/utrace.c 2010-09-22 01:54:24.000000000 +0200
@@ -1139,7 +1139,9 @@ int utrace_control(struct task_struct *t
target->utrace_flags &= ~ENGINE_STOP;
mark_engine_detached(engine);
reset = reset || utrace_do_stop(target, utrace);
- if (!reset) {
+ if (reset) {
+ user_disable_single_step(target);
+ } else {
/*
* As in utrace_set_events(), this barrier ensures
* that our engine->flags changes have hit before we
