Re: Warnings/memory corruption in perf intel events

[Peter Zijlstra <peterz@infradead.org>]

On Sun, Aug 09, 2015 at 03:46:48PM -0400, Sasha Levin wrote:
> Hi all,
> 
> While fuzzing with trinity inside a KVM tools guest running -next I've stumbled on the following:


> [424256.911563] ==================================================================
> [424256.913989] BUG: KASan: use after free in intel_get_event_constraints+0xdb0/0xf90 at addr ffff8801741a70e9
> [424256.917044] Read of size 1 by task trinity-c162/20397
> [424256.918696] =============================================================================
> [424256.921309] BUG kmalloc-2048 (Tainted: G        W      ): kasan: bad access detected


Quite the puzzle that, and I'm not entirely sure I see how. The WARNs
preceding this aren't giving me much confidence either, I've yet to find
a way for them to happen.

That said, the only dynamically allocated memory here is managed on CPU
hotplug. And we appear to consistently return NOTIFY_BAD if an
allocation there fails, which should preclude the CPU hotplug from
completing and therefore preclude events from forming on that CPU.

We do however appear to fail to NULL all our pointers, and even miss one
kfree() in the error paths there (the constraint_list allocation fail,
fails to free the shared_regs one).

Rework that code such that we kfree() all and always NULL our pointers
after free.

---
 arch/x86/kernel/cpu/perf_event_intel.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index a478e3c4cc3f..3f124d553c5a 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -2758,7 +2758,7 @@ static int intel_pmu_cpu_prepare(int cpu)
 	if (x86_pmu.extra_regs || x86_pmu.lbr_sel_map) {
 		cpuc->shared_regs = allocate_shared_regs(cpu);
 		if (!cpuc->shared_regs)
-			return NOTIFY_BAD;
+			goto err;
 	}
 
 	if (x86_pmu.flags & PMU_FL_EXCL_CNTRS) {
@@ -2766,18 +2766,27 @@ static int intel_pmu_cpu_prepare(int cpu)
 
 		cpuc->constraint_list = kzalloc(sz, GFP_KERNEL);
 		if (!cpuc->constraint_list)
-			return NOTIFY_BAD;
+			goto err_shared_regs;
 
 		cpuc->excl_cntrs = allocate_excl_cntrs(cpu);
-		if (!cpuc->excl_cntrs) {
-			kfree(cpuc->constraint_list);
-			kfree(cpuc->shared_regs);
-			return NOTIFY_BAD;
-		}
+		if (!cpuc->excl_cntrs)
+			goto err_constraint_list;
+
 		cpuc->excl_thread_id = 0;
 	}
 
 	return NOTIFY_OK;
+
+err_constraint_list:
+	kfree(cpuc->constraint_list);
+	cpuc->constraint_list = NULL;
+
+err_shared_regs:
+	kfree(cpuc->shared_regs);
+	cpuc->shared_regs = NULL;
+
+err:
+	return NOTIFY_BAD;
 }
 
 static void intel_pmu_cpu_starting(int cpu)