linux-kernel - Re: Warnings/memory corruption in perf intel events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150810112507.GW16853@twins.programming.kicks-ass.net>
Date:	Mon, 10 Aug 2015 13:25:07 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Sasha Levin <sasha.levin@...cle.com>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	LKML <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: Warnings/memory corruption in perf intel events

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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/