public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Clark Williams <williams@redhat.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	Ravi Bangoria <ravi.bangoria@linux.ibm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Andi Kleen <ak@linux.intel.com>, David Ahern <dsahern@gmail.com>,
	David Carrillo-Cisneros <davidcc@google.com>,
	Jin Yao <yao.jin@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 19/19] perf tools: Fix crash caused by accessing feat_ops[HEADER_LAST_FEATURE]
Date: Mon, 25 Jun 2018 14:40:45 -0300	[thread overview]
Message-ID: <20180625174045.25765-20-acme@kernel.org> (raw)
In-Reply-To: <20180625174045.25765-1-acme@kernel.org>

From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

perf_event__process_feature() accesses feat_ops[HEADER_LAST_FEATURE]
which is not defined and thus perf is crashing. HEADER_LAST_FEATURE is
used as an end marker for the perf report but it's unused for perf
script/annotate. Ignore HEADER_LAST_FEATURE for perf script/annotate,
just like it is done in 'perf report'.

Before:
  # perf record -o - ls | perf script
  <SNIP 'ls' output>
  Segmentation fault (core dumped)
  #

After:
  # perf record -o - ls | perf script
  <SNIP 'ls' output>
  Segmentation fault (core dumped)
  ls 7031 4392.099856:  250000 cpu-clock:uhH:  7f5e0ce7cd60
  ls 7031 4392.100355:  250000 cpu-clock:uhH:  7f5e0c706ef7
  #

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: David Carrillo-Cisneros <davidcc@google.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Fixes: 57b5de463925 ("perf report: Support forced leader feature in pipe mode")
Link: http://lkml.kernel.org/r/20180625124220.6434-4-ravi.bangoria@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c | 11 ++++++++++-
 tools/perf/builtin-report.c   |  3 ++-
 tools/perf/builtin-script.c   | 11 ++++++++++-
 tools/perf/util/header.c      |  2 +-
 4 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 5eb22cc56363..8180319285af 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -283,6 +283,15 @@ static int process_sample_event(struct perf_tool *tool,
 	return ret;
 }
 
+static int process_feature_event(struct perf_tool *tool,
+				 union perf_event *event,
+				 struct perf_session *session)
+{
+	if (event->feat.feat_id < HEADER_LAST_FEATURE)
+		return perf_event__process_feature(tool, event, session);
+	return 0;
+}
+
 static int hist_entry__tty_annotate(struct hist_entry *he,
 				    struct perf_evsel *evsel,
 				    struct perf_annotate *ann)
@@ -471,7 +480,7 @@ int cmd_annotate(int argc, const char **argv)
 			.attr	= perf_event__process_attr,
 			.build_id = perf_event__process_build_id,
 			.tracing_data   = perf_event__process_tracing_data,
-			.feature	= perf_event__process_feature,
+			.feature	= process_feature_event,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
 		},
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index cdb5b6949832..c04dc7b53797 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -217,7 +217,8 @@ static int process_feature_event(struct perf_tool *tool,
 	}
 
 	/*
-	 * All features are received, we can force the
+	 * (feat_id = HEADER_LAST_FEATURE) is the end marker which
+	 * means all features are received, now we can force the
 	 * group if needed.
 	 */
 	setup_forced_leader(rep, session->evlist);
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index ad2ac1300420..568ddfac3213 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3044,6 +3044,15 @@ int process_cpu_map_event(struct perf_tool *tool __maybe_unused,
 	return set_maps(script);
 }
 
+static int process_feature_event(struct perf_tool *tool,
+				 union perf_event *event,
+				 struct perf_session *session)
+{
+	if (event->feat.feat_id < HEADER_LAST_FEATURE)
+		return perf_event__process_feature(tool, event, session);
+	return 0;
+}
+
 #ifdef HAVE_AUXTRACE_SUPPORT
 static int perf_script__process_auxtrace_info(struct perf_tool *tool,
 					      union perf_event *event,
@@ -3088,7 +3097,7 @@ int cmd_script(int argc, const char **argv)
 			.attr		 = process_attr,
 			.event_update   = perf_event__process_event_update,
 			.tracing_data	 = perf_event__process_tracing_data,
-			.feature	 = perf_event__process_feature,
+			.feature	 = process_feature_event,
 			.build_id	 = perf_event__process_build_id,
 			.id_index	 = perf_event__process_id_index,
 			.auxtrace_info	 = perf_script__process_auxtrace_info,
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 59fcc790c865..653ff65aa2c3 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -3464,7 +3464,7 @@ int perf_event__process_feature(struct perf_tool *tool,
 		pr_warning("invalid record type %d in pipe-mode\n", type);
 		return 0;
 	}
-	if (feat == HEADER_RESERVED || feat > HEADER_LAST_FEATURE) {
+	if (feat == HEADER_RESERVED || feat >= HEADER_LAST_FEATURE) {
 		pr_warning("invalid record type %d in pipe-mode\n", type);
 		return -1;
 	}
-- 
2.14.3


  parent reply	other threads:[~2018-06-25 17:42 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-25 17:40 [GIT PULL 00/19] perf/urgent fixes for 4.18 Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 01/19] perf record: Support s390 random socket_id assignment Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 02/19] perf test session topology: Fix test on s390 Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 03/19] perf report powerpc: Fix crash if callchain is empty Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 04/19] perf tests: Add event parsing error handling to parse events test Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 05/19] perf tests: Add valid callback for parse-events test Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 06/19] perf intel-pt: Fix packet decoding of CYC packets Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 07/19] tools headers uapi: Synchronize drm/drm.h Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 08/19] perf tools: Update x86's syscall_64.tbl, adding 'io_pgetevents' and 'rseq' Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 09/19] tools include powerpc: Update arch/powerpc/include/uapi/asm/unistd.h copy to get 'rseq' syscall Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 10/19] tools include uapi: Update if_link.h to pick IFLA_{BRPORT_ISOLATED,VXLAN_TTL_INHERIT} Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 11/19] tools include uapi: Synchronize bpf.h with the kernel Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 12/19] perf tools: Fix a clang 7.0 compilation error Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 13/19] perf alias: Remove trailing newline when reading sysfs files Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 14/19] perf alias: Rebuild alias expression string to make it comparable Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 15/19] perf stat: Remove duplicate event counting Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 16/19] perf bench: Fix numa report output code Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 17/19] perf script: Add missing output fields in a hint Arnaldo Carvalho de Melo
2018-06-25 17:40 ` [PATCH 18/19] perf script: Fix crash because of missing evsel->priv Arnaldo Carvalho de Melo
2018-06-25 17:40 ` Arnaldo Carvalho de Melo [this message]
2018-06-25 20:26 ` [GIT PULL 00/19] perf/urgent fixes for 4.18 Arnaldo Carvalho de Melo
2018-06-26  6:46 ` Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180625174045.25765-20-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=davidcc@google.com \
    --cc=dsahern@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=ravi.bangoria@linux.ibm.com \
    --cc=williams@redhat.com \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox