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

[Roland McGrath]

> 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 because utrace_reset can be called when the tracee is
not stopped, and it's not safe to call user_*_step then (with the one
caveat that the SIGKILL race is OK).

> The 2nd one which changes utrace_control(DETACH),
> 
> 	@@ -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
> 
> It doesn't cover the case when the TIF_SINGLESTEP tracee detaches
> itself, but a) currently the are no such engines, and b) it looks
> more clean.

If we're trying to meet the advertised API--how it's documented now is
the only reasonable thing if we are actually fixing things properly at
all--we do need to cover the case where the only engine to have
requested single-step earlier then later returns UTRACE_DETACH from a
callback.  That is, if that engine could actually know that the tracee
never reached user mode between its use of UTRACE_SINGLESTEP and its
later UTRACE_DETACH.

> 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.  

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.  In that particular case, it shouldn't be a problem, since
after its report_signal we will always hit finish_resume_report later.
But we should think about if there are any holes of a similar nature.

> 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.


Thanks,
Roland