[PATCH] utrace: utrace_reset() should clear TIF_SINGLESTEP if no more engines

[Oleg Nesterov]

On 10/14, Roland McGrath wrote:
>
> > The 1st patch I sent initially:
> >
> > 	--- kstub/kernel/utrace.c~10_utrace_reset_should_clear_ss	2010-09-20 03:55:12.000000000 +0200
> > 	+++ kstub/kernel/utrace.c	2010-09-20 21:33:47.000000000 +0200
> > 	@@ -709,6 +709,7 @@ static bool utrace_reset(struct task_str
> > 			 */
> > 			utrace->resume = UTRACE_RESUME;
> > 			utrace->signal_handler = 0;
> > 	+		user_disable_single_step(task);
> > 		}
> >
> > 		/*
> >
> > It clears TIF_SINGLESTEP when the last engine detaches.
>
> This we didn't want

Agreed, I mentioned this patch only to remind the options we
already discussed.

But,

> because utrace_reset can be called when the tracee is
> not stopped, and it's not safe to call user_*_step then

Hmm. If task is not stopped then it is current (except
utrace_control(DETACH) can play with the dying task).

If this is not safe, what about finish_resume_report() ?

> > However, I am starting to think that if you prefer this change we
> > have a better option. Instead of duplicating UTRACE_RESUME logic,
> > perhaps the patch below makes more sense?
>
> Hmm, that does seem like it might be pretty reasonable.  If your
> engine had ever permitted the tracee to get back to user mode after
> requesting UTRACE_*STEP, then it's just desserts to cause that SIGTRAP
> regardless of whether other engines might have kept it stopped in
> actuality.

Not sure I understand what you mean.

But this patch looks reasonable to me too. It translates the current
"There might not be another report before it just resumes" comment in
utrace_control(UTRACE_RESUME) into the plain C code. And solves the
original problem with SINGLESTEP after DETACH (but not all problems).

> But, there might be plausible corners where an engine really could know
> reliably (without outside knowledge of what other engines might be
> doing) that the tracee can't really have been back to user mode yet.
> For example, if it checked signal_pending(tracee) before it used
> UTRACE_*STEP, then it could know for sure that its report_signal
> callback will have been called before it ever got to user mode, so if
> that callback returns UTRACE_DETACH, it better not be left with stepping
> enabled.

Yes, this patch can't help if the tracee detaches itself. We need
more changes, but so far these changes are not clear to me. Plus
->resume == *STEP* "leak" we are discussing in another thread.

> > Please tell me which fix you prefer? Or just commit the fix you
> > like more.
>
> I'm only going to merge a specific commit of yours that you have tested.

I tried to test them all. But I think that your review is much more
useful, apart from ptrace-tests I do not know what else I can use
for the testing.

I'll send the last patch in a minute.

Oleg.