(on top of utrace-core.patch)
The comment in utrace_maybe_reap() correctly explains why
utrace_attach_task/utrace_control/etc can't modify or use
attaching/attached lists. But find_matching_engine() can
scan ->attached under utrace->lock without any checks, it
can race with utrace_maybe_reap() destroying list nodes.
Change utrace_maybe_reap() to empty ->attached before it
drops utrace->lock, update the comments a bit.
Reported-by: CAI Qian <caiq...@redhat.com>
Signed-off-by: Oleg Nesterov <o...@redhat.com>
Signed-off-by: Roland McGrath <rol...@redhat.com>
---
kernel/utrace.c | 23 ++++++++++++++++-------
1 files changed, 16 insertions(+), 7 deletions(-)
--- V1/kernel/utrace.c~8_REAP_FIND_RACE 2009-12-18 01:58:37.000000000 +0100
+++ V1/kernel/utrace.c 2010-01-21 17:31:18.000000000 +0100
@@ -1,7 +1,7 @@
/*
* utrace infrastructure interface for debugging user processes
*
- * Copyright (C) 2006-2009 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2006-2010 Red Hat, Inc. All rights reserved.
*
* This copyrighted material is made available to anyone wishing to use,
* modify, copy, or redistribute it subject to the terms and conditions
@@ -859,6 +859,7 @@ void utrace_maybe_reap(struct task_struc
bool reap)
{
struct utrace_engine *engine, *next;
+ struct list_head attached;
spin_lock(&utrace->lock);
@@ -897,16 +898,24 @@ void utrace_maybe_reap(struct task_struc
}
/*
- * utrace_add_engine() checks ->utrace_flags != 0.
- * Since @utrace->reap is set, nobody can set or clear
- * UTRACE_EVENT(REAP) in @engine->flags or change
- * @engine->ops, and nobody can change @utrace->attached.
+ * utrace_add_engine() checks ->utrace_flags != 0. Since
+ * @utrace->reap is set, nobody can set or clear UTRACE_EVENT(REAP)
+ * in @engine->flags or change @engine->ops and nobody can change
+ * @utrace->attached after we drop the lock.
*/
target->utrace_flags = 0;
- splice_attaching(utrace);
+
+ /*
+ * We clear out @utrace->attached before we drop the lock so
+ * that find_matching_engine() can't come across any old engine
+ * while we are busy tearing it down.
+ */
+ list_replace_init(&utrace->attached, &attached);
+ list_splice_tail_init(&utrace->attaching, &attached);
+
spin_unlock(&utrace->lock);
- list_for_each_entry_safe(engine, next, &utrace->attached, entry) {
+ list_for_each_entry_safe(engine, next, &attached, entry) {
if (engine->flags & UTRACE_EVENT(REAP))
engine->ops->report_reap(engine, target);
