From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754122AbbHJLZR (ORCPT ); Mon, 10 Aug 2015 07:25:17 -0400 Received: from bombadil.infradead.org ([198.137.202.9]:39046 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752800AbbHJLZP (ORCPT ); Mon, 10 Aug 2015 07:25:15 -0400 Date: Mon, 10 Aug 2015 13:25:07 +0200 From: Peter Zijlstra To: Sasha Levin Cc: Ingo Molnar , Arnaldo Carvalho de Melo , Thomas Gleixner , "H. Peter Anvin" , LKML , "x86@kernel.org" , Stephane Eranian Subject: Re: Warnings/memory corruption in perf intel events Message-ID: <20150810112507.GW16853@twins.programming.kicks-ass.net> References: <55C7AE28.30706@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <55C7AE28.30706@oracle.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.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)