[PATCH] UTRACE_STOP race condition?

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

renzo

On Fri, Feb 13, 2009 at 09:29:25PM +0100, Renzo Davoli wrote:
> I have now a complete patch that seems to be quite stable.
> At least Kmview have passed through the tests without getting stuck randomly for the race condition.
> 
---
--- kernel/utrace.c.mcgrath	2009-03-05 15:09:57.000000000 +0100
+++ kernel/utrace.c	2009-03-06 11:20:48.000000000 +0100
@@ -369,6 +369,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.
@@ -378,6 +385,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
@@ -399,7 +407,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);
 
 	/*
@@ -625,6 +649,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)
 {
@@ -641,6 +666,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
@@ -891,6 +931,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);
 		}
 	}
@@ -1110,6 +1154,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;