Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933172AbcKORFB convert rfc822-to-8bit (ORCPT ); Tue, 15 Nov 2016 12:05:01 -0500 Received: from mga03.intel.com ([134.134.136.65]:46912 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752270AbcKORE5 (ORCPT ); Tue, 15 Nov 2016 12:04:57 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,495,1473145200"; d="scan'208";a="901691323" From: "Liang, Kan" To: Peter Zijlstra , Vince Weaver CC: "linux-kernel@vger.kernel.org" , Ingo Molnar , Arnaldo Carvalho de Melo , "davej@codemonkey.org.uk" , "dvyukov@google.com" , Stephane Eranian Subject: RE: perf: fuzzer KASAN slab-out-of-bounds in snb_uncore_imc_event_del Thread-Topic: perf: fuzzer KASAN slab-out-of-bounds in snb_uncore_imc_event_del Thread-Index: AQHSPxWB0DPNSwBXtUqA0AWmqeHb66DZBgaAgACH/ICAALYycA== Date: Tue, 15 Nov 2016 17:04:23 +0000 Message-ID: <37D7C6CF3E00A74B8858931C1DB2F07750C9FF81@SHSMSX103.ccr.corp.intel.com> References: <20161115140413.GK3142@twins.programming.kicks-ass.net> In-Reply-To: <20161115140413.GK3142@twins.programming.kicks-ass.net> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzY5ZGE2OGQtZmQ0Ny00ZTI1LWE5ZTUtOGYwNDc2YzlhYTUyIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Ikc1VWlBQ3lLSXJtbjNJMmZLdHRXcXFqUzFsc3F5aG5MUnNuaHNrVGFuenc9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3850 Lines: 115 > > On Tue, Nov 15, 2016 at 12:57:31AM -0500, Vince Weaver wrote: > > On Mon, 14 Nov 2016, Vince Weaver wrote: > > > > > Anyway as per the suggestion at Linux Plumbers I enabled KASAN and > > > on my haswell machine it falls over in a few minutes of running the > perf_fuzzer. > > > > > > [ 205.740194] > > > > =============================================================== > === > > > [ 205.748005] BUG: KASAN: slab-out-of-bounds in > > > snb_uncore_imc_event_del+0x6c/0xa0 at addr ffff8800caa43768 [ > > > 205.758324] Read of size 8 by task perf_fuzzer/6618 [ 205.763589] > > > CPU: 0 PID: 6618 Comm: perf_fuzzer Not tainted 4.9.0-rc5 #4 [ > > > 205.770721] Hardware name: LENOVO 10AM000AUS/SHARKBAY, BIOS > > > FBKT72AUS 01/26/2014 [ 205.778689] ffff8800c3c479b8 > > > ffffffff816bb796 ffff88011ec00600 ffff8800caa43580 [ 205.786759] > > > ffff8800c3c479e0 ffffffff812fb961 ffff8800c3c47a78 ffff8800caa43580 > [ 205.794850] ffff8800caa43580 ffff8800c3c47a68 ffffffff812fbbd8 > ffff8800c3c47a28 [ 205.802911] Call Trace: > > > [ 205.805559] [] dump_stack+0x63/0x8d [ > > > 205.811135] [] kasan_object_err+0x21/0x70 [ > > > 205.817267] [] kasan_report_error+0x1d8/0x4c0 [ > > > 205.823752] [] ? __lock_is_held+0x75/0xc0 [ > > > 205.829868] [] ? > > > snb_uncore_imc_read_counter+0x42/0x50 > > > [ 205.837198] [] ? > > > uncore_perf_event_update+0xe2/0x160 > > > [ 205.844337] [] kasan_report+0x39/0x40 [ > > > 205.850085] [] ? > > > snb_uncore_imc_event_del+0x6c/0xa0 > > > > The best I can tell this maps to: > > > > static void snb_uncore_imc_event_del(struct perf_event *event, int > > flags) { > > struct intel_uncore_box *box = uncore_event_to_box(event); > > int i; > > > > snb_uncore_imc_event_stop(event, PERF_EF_UPDATE); > > > > for (i = 0; i < box->n_events; i++) { > > >>> if (event == box->event_list[i]) { > > --box->n_events; > > break; > > } > > } > > } > > > > Can this code be right? Does it actually remove the event? > > The similar code in > > > > static void uncore_pmu_event_del(struct perf_event *event, int flags) > > > > .... > > > > for (i = 0; i < box->n_events; i++) { > > if (event == box->event_list[i]) { > > uncore_put_event_constraint(box, event); > > > > for (++i; i < box->n_events; i++) > > box->event_list[i - 1] = > > box->event_list[i]; > > > > --box->n_events; > > break; > > } > > } > > > > > > seems like it is more likely to be correct. > > Kan, can you look at this? For client IMC, there is no generic counters. Current implementation defines its own fixed free running counters. event_list and n_events are unused. I think we can just remove them. Vince, could you please try the patch as below? ------ diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c index 81195cc..a3dcc12 100644 --- a/arch/x86/events/intel/uncore_snb.c +++ b/arch/x86/events/intel/uncore_snb.c @@ -490,24 +490,12 @@ static int snb_uncore_imc_event_add(struct perf_event *event, int flags) snb_uncore_imc_event_start(event, 0); - box->n_events++; - return 0; } static void snb_uncore_imc_event_del(struct perf_event *event, int flags) { - struct intel_uncore_box *box = uncore_event_to_box(event); - int i; - snb_uncore_imc_event_stop(event, PERF_EF_UPDATE); - - for (i = 0; i < box->n_events; i++) { - if (event == box->event_list[i]) { - --box->n_events; - break; - } - } } int snb_pci2phy_map_init(int devid)