[PATCH] fix mark_engine_detached() vs start_callback() race

Oleg Nesterov oleg at redhat.com
Wed Aug 18 16:32:33 UTC 2010


Suppose that engine->flags == UTRACE_EVENT(EXEC), QUIESCE bit is not set.

	1. start_callback() reads want = engine->flags (== EXEC)

	2. mark_engine_detached() sets engine->ops = &utrace_detached_ops

	3. start_callback() gets ops = utrace_detached_ops

After that start_callback() skips "if (want & UTRACE_EVENT(QUIESCE))"
block and returns utrace_detached_ops, then ->report_exec == NULL
will be called.

This is the minimal temporary ugly fix for now, we should certainly
cleanup and simplify this logic. The barriers in mark_engine_detached()
and in start_callback() can't help and should be removed. If we ignore
utrace_get_signal() we do not even need utrace_detached_quiesce(),
start_callback() could just do

	ops = engine->ops;

	if (ops == utrace_detached_ops) {
		report->detaches = true;
		return NULL;
	}

I think in the longer term mark_engine_detached() should not change
engine->flags at all but add QUIESCE to ->utrace_flags. However, this
breaks utrace_maybe_reap(reap => true) and we should avoid the race
with finish_callback() which clears ->reporting after report_quiesce().

A bit off-topic, but I don't think finish_callback() should check
engine->ops == &utrace_detached_ops before return. Instead we should
change finish_callback_report() to return the boolean. We shouldn't
return true without setting report->detaches.

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

 kernel/utrace.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- kstub/kernel/utrace.c~8_fix_mark_detached_without_quiesce	2010-08-18 16:46:08.000000000 +0200
+++ kstub/kernel/utrace.c	2010-08-18 17:47:53.000000000 +0200
@@ -1522,7 +1522,7 @@ static const struct utrace_engine_ops *s
 	smp_rmb();
 	ops = engine->ops;
 
-	if (want & UTRACE_EVENT(QUIESCE)) {
+	if ((want & UTRACE_EVENT(QUIESCE)) || ops == &utrace_detached_ops) {
 		if (finish_callback(task, utrace, report, engine,
 				    (*ops->report_quiesce)(report->action,
 							   engine, event)))




More information about the utrace-devel mailing list