From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752132AbbIGM4H (ORCPT ); Mon, 7 Sep 2015 08:56:07 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:60705 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751450AbbIGM4C (ORCPT ); Mon, 7 Sep 2015 08:56:02 -0400 From: Wang Nan To: , CC: , , , Wang Nan , Adrian Hunter , Andi Kleen , Jiri Olsa , Namhyung Kim , Stephane Eranian , "Arnaldo Carvalho de Melo" Subject: [PATCH] perf report: Fix invalid memory accessing Date: Mon, 7 Sep 2015 12:51:55 +0000 Message-ID: <1441630315-189525-1-git-send-email-wangnan0@huawei.com> X-Mailer: git-send-email 1.8.3.4 MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.107.193.248] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Commit e1e499aba570a2ea84d29822b7ea637ac41d9a51 (perf tools: Add processor socket info to hist_entry and addr_location) reads env->cpu array for each sample using index al.cpu. However, al.cpu can be -1 if sample doesn't select PERF_SAMPLE_CPU. Also, env->cpu can be invalid if feature CPU_TOPOLOGY not selected. We should validate env->cpu and al.cpu before setting al.socket. Signed-off-by: Wang Nan Cc: Kan Liang Cc: Adrian Hunter Cc: Andi Kleen Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Cc: Arnaldo Carvalho de Melo --- Although theoretically CPU_TOPOLOGY feature should always be selected by 'perf record', I did generate a perf.data without that feature. It has header like this: # perf report -i ./bad.perf.data --header-only # ======== # captured on: Thu Jan 8 09:30:15 2009 # hostname : localhost # os release : 3.10.49-gd672fc4 # perf version : 4.2.gc9df # arch : aarch64 # nrcpus online : 8 # nrcpus avail : 8 # total memory : 1850768 kB # cmdline : /system/bin/perf record -e sync:sync_timeline -e kgsl:kgsl_register_event -g -a sleep 5 # event : name = sync:sync_timeline, , id = { 1107, 1108, 1109, 1110, 1111, 1112 }, type = 2, size = 112, config = 0x3e7, { sample_period, sample_freq } = 1, sample_type = IP|TID|TIME|CALLCHAIN|ID|CPU|PERIOD|RAW, read_format = ID, disabled = 1, inherit = 1, mmap = 1, comm = 1, task = 1, sample_id_all = 1, exclude_guest = 1, mmap2 = 1, comm_exec = 1 # event : name = kgsl:kgsl_register_event, , id = { 1113, 1114, 1115, 1116, 1117, 1118 }, type = 2, size = 112, config = 0x350, { sample_period, sample_freq } = 1, sample_type = IP|TID|TIME|CALLCHAIN|ID|CPU|PERIOD|RAW, read_format = ID, disabled = 1, inherit = 1, sample_id_all = 1, exclude_guest = 1 # pmu mappings: cpu = 4, software = 1, tracepoint = 2 # ======== # It should be: # ======== # captured on: Thu Jan 8 11:26:41 2009 ... # HEADER_CPU_TOPOLOGY info available, use -I to display # pmu mappings: cpu = 4, software = 1, tracepoint = 2 # ======== However, bad perf.data appears randomly. I can't stably reproduce it, so I guess there might have another invalid memory accessing. --- tools/perf/builtin-report.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 4b43245..16d097d 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -158,8 +158,16 @@ static int process_sample_event(struct perf_tool *tool, return -1; } - /* read socket id from perf.data for perf report */ - al.socket = env->cpu[al.cpu].socket_id; + /* + * read socket id from perf.data for perf report + * al.cpu is invalid if PERF_SAMPLE_CPU is not selected by this + * sample. + * env->cpu is invalid if CPU_TOPOLOGY feature is not set in + * header. + */ + al.socket = -1; + if (env->cpu && al.cpu >= 0) + al.socket = env->cpu[al.cpu].socket_id; if (rep->hide_unresolved && al.sym == NULL) goto out_put; -- 1.8.3.4