From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Chris Phlipot <cphlipot0@gmail.com>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 04/30] perf script: Fix segfault when printing callchains
Date: Wed, 27 Apr 2016 11:30:46 -0300 [thread overview]
Message-ID: <1461767472-8827-5-git-send-email-acme@kernel.org> (raw)
In-Reply-To: <1461767472-8827-1-git-send-email-acme@kernel.org>
From: Chris Phlipot <cphlipot0@gmail.com>
This fixes a bug caused by an unitialized callchain cursor. The crash
frist appeared in:
6f736735e30f ("perf evsel: Require that callchains be resolved before
calling fprintf_{sym,callchain}")
The callchain cursor is a struct that contains pointers, that when
uninitialized will cause unpredictable behavior (usually a crash)
when trying to append to the callchain.
The existing implementation has the following issues:
1. The callchain cursor used is not initialized, resulting in
unpredictable behavior when used.
2. The cursor is declared on the stack. Even if it is properly initalized,
the implmentation will leak memory when the function returns,
since all the references to the callchain_nodes allocated by
callchain_cursor_append will be lost when the cursor goes out of
scope.
3. Storing the cursor on the stack is inefficient. Even if memory is
properly freed when it goes out of scope, a performance penalty
will be incurred due to reallocation of callchain nodes.
callchain_cursor_append is designed to avoid these reallocations
when an existing cursor is reused.
This patch fixes the crash by replacing cursor_callchain with a reference
to the global callchain_cursor which also resolves all 3 issues mentioned
above.
How to reproduce the crash:
$ perf record --call-graph=dwarf stress -t 1 -c 1
$ perf script > /dev/null
Segfault
Signed-off-by: Chris Phlipot <cphlipot0@gmail.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 6f736735e30f ("perf evsel: Require that callchains be resolved before calling fprintf_{sym,callchain}")
Link: http://lkml.kernel.org/r/1461119531-2529-1-git-send-email-cphlipot0@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-script.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 5099740aa50b..f43b0c6f88f4 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -570,12 +570,12 @@ static void print_sample_bts(struct perf_sample *sample,
/* print branch_from information */
if (PRINT_FIELD(IP)) {
unsigned int print_opts = output[attr->type].print_ip_opts;
- struct callchain_cursor *cursor = NULL, cursor_callchain;
+ struct callchain_cursor *cursor = NULL;
if (symbol_conf.use_callchain && sample->callchain &&
- thread__resolve_callchain(al->thread, &cursor_callchain, evsel,
+ thread__resolve_callchain(al->thread, &callchain_cursor, evsel,
sample, NULL, NULL, scripting_max_stack) == 0)
- cursor = &cursor_callchain;
+ cursor = &callchain_cursor;
if (cursor == NULL) {
putchar(' ');
@@ -789,12 +789,12 @@ static void process_event(struct perf_script *script,
printf("%16" PRIu64, sample->weight);
if (PRINT_FIELD(IP)) {
- struct callchain_cursor *cursor = NULL, cursor_callchain;
+ struct callchain_cursor *cursor = NULL;
if (symbol_conf.use_callchain && sample->callchain &&
- thread__resolve_callchain(al->thread, &cursor_callchain, evsel,
+ thread__resolve_callchain(al->thread, &callchain_cursor, evsel,
sample, NULL, NULL, scripting_max_stack) == 0)
- cursor = &cursor_callchain;
+ cursor = &callchain_cursor;
putchar(cursor ? '\n' : ' ');
sample__fprintf_sym(sample, al, 0, output[attr->type].print_ip_opts, cursor, stdout);
--
2.5.5
next prev parent reply other threads:[~2016-04-27 14:31 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-27 14:30 [GIT PULL 00/30] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 01/30] perf buildid: Fix off-by-one in write_buildid() Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 02/30] perf trace: Extract evsel contructor from perf_evlist__add_pgfault Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 03/30] perf trace: Make --pf maj/min/all use callchains too Arnaldo Carvalho de Melo
2016-04-27 14:30 ` Arnaldo Carvalho de Melo [this message]
2016-04-27 14:30 ` [PATCH 05/30] perf trace: Make --event honour --min-stack too Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 06/30] perf trace: Make --pf " Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 07/30] perf evlist: Decode perf_event_attr->branch_sample_type Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 08/30] perf tools: Make the x86 clean quiet Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 09/30] tools build: Fix perf_clean target Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 10/30] perf tools: Remove duplicate const qualifier Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 11/30] perf tests: Replace assignment with comparison on assert check Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 12/30] perf bench futex: Simplify wrapper for LOCK_PI Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 13/30] perf intel-pt: Fix off-by-one comparison on maximum code Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 14/30] perf hists: Clear dummy entry accumulated period Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 15/30] perf evlist: Enforce ring buffer reading Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 16/30] perf probe: Close target file on error path Arnaldo Carvalho de Melo
2016-04-27 14:30 ` [PATCH 17/30] perf tools: Add lsdir() helper to read a directory Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 18/30] perf probe: Let probe_file__add_event return 0 if succeeded Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 19/30] perf probe: Set default kprobe group name if it is not given Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 20/30] perf trace: Move perf_flags beautifier to tools/perf/trace/beauty/ Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 21/30] perf trace: Do not beautify the 'pid' parameter as a simple integer Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 22/30] tools lib api fs: Add helper to read string from procfs file Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 23/30] perf thread: Introduce method to set comm from /proc/pid/self Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 24/30] perf trace: Read thread's COMM from /proc when not set Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 25/30] perf probe: Fix offline module name missmatch issue Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 26/30] perf probe: Fix module probe issue if no dwarf support Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 27/30] perf tools: Update x86's syscall_64.tbl, adding preadv2 & pwritev2 Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 28/30] perf bench: Remove one more die() call Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 29/30] perf core: Allow setting up max frame stack depth via sysctl Arnaldo Carvalho de Melo
2016-04-27 14:31 ` [PATCH 30/30] perf tools: Set the maximum allowed stack from /proc/sys/kernel/perf_event_max_stack Arnaldo Carvalho de Melo
2016-04-27 15:03 ` [GIT PULL 00/30] perf/core improvements and fixes 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=1461767472-8827-5-git-send-email-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=cphlipot0@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
/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;
as well as URLs for NNTP newsgroup(s).