[PATCH 1/2] UTRACE_STOP race condition (updated)

[Renzo Davoli]

Dear Roland, dear utrace developers,

I have updated my patch #1 (it solves the race condition on utrace_stop but
not the nesting issue) for the latest version of utrace.

I am trying to get the patches updated downloading, compiling and testing
the fixes every week or so... 
Things would be easier if these patch could be merged in the mainstream ;-)

renzo
----
diff -Naur linux-2.6.29-rc7-git5-utrace/kernel/utrace.c linux-2.6.29-rc7-git5-utrace-p1/kernel/utrace.c
--- linux-2.6.29-rc7-git5-utrace/kernel/utrace.c	2009-03-12 11:00:09.000000000 +0100
+++ linux-2.6.29-rc7-git5-utrace-p1/kernel/utrace.c	2009-03-12 11:05:50.000000000 +0100
@@ -376,6 +376,13 @@
 	return killed;
 }
 
+static void mark_engine_wants_stop(struct utrace_engine *engine);
+static void clear_engine_wants_stop(struct utrace_engine *engine);
+static bool engine_wants_stop(struct utrace_engine *engine);
+static void mark_engine_wants_resume(struct utrace_engine *engine);
+static void clear_engine_wants_resume(struct utrace_engine *engine);
+static bool engine_wants_resume(struct utrace_engine *engine);
+
 /*
  * Perform %UTRACE_STOP, i.e. block in TASK_TRACED until woken up.
  * @task == current, @utrace == current->utrace, which is not locked.
@@ -385,6 +392,7 @@
 static bool utrace_stop(struct task_struct *task, struct utrace *utrace)
 {
 	bool killed;
+	struct utrace_engine *engine, *next;
 
 	/*
 	 * @utrace->stopped is the flag that says we are safely
@@ -406,7 +414,23 @@
 		return true;
 	}
 
-	utrace->stopped = 1;
+	/* final check: it is really needed to stop? */
+	list_for_each_entry_safe(engine, next, &utrace->attached, entry) {
+		if ((engine->ops != &utrace_detached_ops) && engine_wants_stop(engine)) {
+			if (engine_wants_resume(engine)) {
+				clear_engine_wants_stop(engine);
+				clear_engine_wants_resume(engine);
+			}
+			else
+				utrace->stopped = 1;
+		}
+	}
+	if (unlikely(!utrace->stopped)) {
+		spin_unlock_irq(&task->sighand->siglock);
+		spin_unlock(&utrace->lock);
+		return false;
+	}
+
 	__set_current_state(TASK_TRACED);
 
 	/*
@@ -632,6 +656,7 @@
  * to record whether the engine is keeping the target thread stopped.
  */
 #define ENGINE_STOP		(1UL << _UTRACE_NEVENTS)
+#define ENGINE_RESUME		(1UL << (_UTRACE_NEVENTS+1))
 
 static void mark_engine_wants_stop(struct utrace_engine *engine)
 {
@@ -648,6 +673,21 @@
 	return (engine->flags & ENGINE_STOP) != 0;
 }
 
+static void mark_engine_wants_resume(struct utrace_engine *engine)
+{
+	engine->flags |= ENGINE_RESUME;
+}
+
+static void clear_engine_wants_resume(struct utrace_engine *engine)
+{
+	engine->flags &= ~ENGINE_RESUME;
+}
+
+static bool engine_wants_resume(struct utrace_engine *engine)
+{
+	return (engine->flags & ENGINE_RESUME) != 0;
+}
+
 /**
  * utrace_set_events - choose which event reports a tracing engine gets
  * @target:		thread to affect
@@ -906,6 +946,10 @@
 			list_move(&engine->entry, &detached);
 		} else {
 			flags |= engine->flags | UTRACE_EVENT(REAP);
+			if (engine_wants_resume(engine)) {
+				clear_engine_wants_stop(engine);
+				clear_engine_wants_resume(engine);
+			}
 			wake = wake && !engine_wants_stop(engine);
 		}
 	}
@@ -1133,6 +1177,7 @@
 		 * There might not be another report before it just
 		 * resumes, so make sure single-step is not left set.
 		 */
+		mark_engine_wants_resume(engine);
 		if (likely(resume))
 			user_disable_single_step(target);
 		break;