Re: use-after-free in __perf_install_in_context

[Peter Zijlstra <peterz@infradead.org>]

On Fri, Dec 04, 2015 at 09:04:35PM +0100, Dmitry Vyukov wrote:
> Hello,
> 
> While running syzkaller fuzzer I am seeing lots of the following
> use-after-free reports. Unfortunately all my numerous attempts to
> reproduce them in a controlled environment failed. They pop up during
> fuzzing periodically (once in several hours in a single VM), but
> whenever I try to stress-replay what happened in the VM before the
> report, the use-after-free does not reproduce. 

Typical that :/

> Can somebody knowledgeable in perf subsystem look at the report? Maybe
> it is possible to figure out what happened based purely on the report.
> I can pretty reliably test any proposed fixes.

So I'm still going over the code, but meanwhile I tried reproducing this
using the perf_fuzzer and some debug code, but no luck with that.

Since you seem to be able to reproduce, could you do a run with the
below patch in to see if it tickles something?


---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 36babfd..58e504c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -872,13 +872,22 @@ static void free_ctx(struct rcu_head *head)
 	kfree(ctx);
 }
 
+#define for_each_task_context_nr(ctxn)					\
+	for ((ctxn) = 0; (ctxn) < perf_nr_task_contexts; (ctxn)++)
+
 static void put_ctx(struct perf_event_context *ctx)
 {
 	if (atomic_dec_and_test(&ctx->refcount)) {
 		if (ctx->parent_ctx)
 			put_ctx(ctx->parent_ctx);
-		if (ctx->task)
+		if (ctx->task) {
+			int ctxn;
+
+			for_each_task_context_nr(ctxn)
+				WARN_ON(ctx->task->perf_event_ctxp[ctxn] == ctx);
+
 			put_task_struct(ctx->task);
+		}
 		call_rcu(&ctx->rcu_head, free_ctx);
 	}
 }
@@ -2649,9 +2658,6 @@ static void perf_pmu_sched_task(struct task_struct *prev,
 static void perf_event_switch(struct task_struct *task,
 			      struct task_struct *next_prev, bool sched_in);
 
-#define for_each_task_context_nr(ctxn)					\
-	for ((ctxn) = 0; (ctxn) < perf_nr_task_contexts; (ctxn)++)
-
 /*
  * Called from scheduler to remove the events of the current task,
  * with interrupts disabled.
@@ -8931,6 +8937,8 @@ again:
 
 		mutex_unlock(&ctx->mutex);
 
+		task->perf_event_ctxp[ctxn] = NULL;
+
 		put_ctx(ctx);
 	}
 }