[PATCH 2/3] fix utrace_control(DETACH) && utrace->death interaction

[Oleg Nesterov]

Suppose that we want to detach the engine and free engine->data. To
avoid the races with our calbacks which can use ->data we should do
either

	err = utrace_set_events(0);
	if (err == ...)
		utrace_barrier();
	utrace_control(DETACH);

or

	err = utrace_control(DETACH);
	if (err = ...)
		utrace_barrier();

Neither variant works if we should care about report_quiesce() or
report_death(). Let's discuss the 2nd case, the 1st one have the
similar problems.

The problem is, utrace_control(DETACH) does nothing and returns
-EALREADY if utrace->death is set, this is not right. We can and
should detach in this case, we only should skip utrace_reset() to
avoid the race with utrace_report_death()->REPORT_CALLBACKS().

This was the main problem I hit during the testing. Consider the
exiting tracee with the valid engine->ops, suppose that
utrace_control(DETACH) is called right after utrace_report_death()
unlocks utrace->lock and before it does REPORT_CALLBACKS().

utrace_control() fails, but utrace_barrier() does not help after
that. It takes get_utrace_lock() successfully and returns 0.
The caller frees engine->data, but utrace_report_death() calls
->report_death() after that.

Kill utrace_control_dead(). Change utrace_control() to always
allow UTRACE_DETACH if engine has the valid ->ops. This means that
mark_engine_detached() can race with REPORT_CALLBACKS() but there
is nothing new. utrace_do_stop() is unnecessary and pointless in
this case, but it doesn't hurt.

Signed-off-by: Oleg Nesterov <oleg at redhat.com>
---

 kernel/utrace.c |   49 ++++++++++---------------------------------------
 1 file changed, 10 insertions(+), 39 deletions(-)

--- kstub/kernel/utrace.c~6_fix_control_dead	2010-08-16 11:17:05.000000000 +0200
+++ kstub/kernel/utrace.c	2010-08-16 11:17:51.000000000 +0200
@@ -928,37 +928,6 @@ void utrace_maybe_reap(struct task_struc
 	}
 }
 
-/*
- * You can't do anything to a dead task but detach it.
- * If release_task() has been called, you can't do that.
- *
- * On the exit path, DEATH and QUIESCE event bits are set only
- * before utrace_report_death() has taken the lock.  At that point,
- * the death report will come soon, so disallow detach until it's
- * done.  This prevents us from racing with it detaching itself.
- *
- * Called only when @target->exit_state is nonzero.
- */
-static inline int utrace_control_dead(struct task_struct *target,
-				      struct utrace *utrace,
-				      enum utrace_resume_action action)
-{
-	lockdep_assert_held(&utrace->lock);
-
-	if (action != UTRACE_DETACH || unlikely(utrace->reap))
-		return -ESRCH;
-
-	if (unlikely(utrace->death))
-		/*
-		 * We have already started the death report.  We can't
-		 * prevent the report_death and report_reap callbacks,
-		 * so tell the caller they will happen.
-		 */
-		return -EALREADY;
-
-	return 0;
-}
-
 /**
  * utrace_control - control a thread being traced by a tracing engine
  * @target:		thread to affect
@@ -1115,18 +1084,20 @@ int utrace_control(struct task_struct *t
 	ret = 0;
 
 	/*
-	 * ->exit_state can change under us, this doesn't matter.
-	 * We do not care about ->exit_state in fact, but we do
-	 * care about ->reap and ->death. If either flag is set,
-	 * we must also see ->exit_state != 0.
+	 * ->exit_state can change under us, this doesn't matter. But,
+	 * if utrace->death is set we must also see ->exit_state != 0,
+	 * this is what we care about.
 	 */
 	if (unlikely(target->exit_state)) {
-		ret = utrace_control_dead(target, utrace, action);
-		if (ret) {
+		/* You can't do anything to a dead task but detach it */
+		if (action != UTRACE_DETACH) {
 			spin_unlock(&utrace->lock);
-			return ret;
+			return -ESRCH;
 		}
-		reset = true;
+
+		/* If it is not set, we can safely do utrace_reset() */
+		if (likely(!utrace->death))
+			reset = true;
 	}
 
 	switch (action) {