Commit a7f720ae authored by Jiri Olsa's avatar Jiri Olsa Committed by Khalid Elmously

perf report: Fix memory corruption in --branch-history mode --branch-history

BugLink: https://bugs.launchpad.net/bugs/1775771

[ Upstream commit e3ebaa46 ]

Jin Yao reported memory corrupton in perf report with
branch info used for stack trace:

  > Following command lines will cause perf crash.

  > perf record -j call -g -a <application>
  > perf report --branch-history
  >
  > *** Error in `perf': double free or corruption (!prev): 0x00000000104aa040 ***
  > ======= Backtrace: =========
  > /lib/x86_64-linux-gnu/libc.so.6(+0x77725)[0x7f6b37254725]
  > /lib/x86_64-linux-gnu/libc.so.6(+0x7ff4a)[0x7f6b3725cf4a]
  > /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7f6b37260abc]
  > perf[0x51b914]
  > perf(hist_entry_iter__add+0x1e5)[0x51f305]
  > perf[0x43cf01]
  > perf[0x4fa3bf]
  > perf[0x4fa923]
  > perf[0x4fd396]
  > perf[0x4f9614]
  > perf(perf_session__process_events+0x89e)[0x4fc38e]
  > perf(cmd_report+0x15d2)[0x43f202]
  > perf[0x4a059f]
  > perf(main+0x631)[0x427b71]
  > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f6b371fd830]
  > perf(_start+0x29)[0x427d89]

For the cumulative output, we allocate the he_cache array based on the
--max-stack option value and populate it with data from 'callchain_cursor'.

The --max-stack option value does not ensure now the limit for number of
callchain_cursor nodes, so the cumulative iter code will allocate smaller array
than it's actually needed and cause above corruption.

I think the --max-stack limit does not apply here anyway, because we add
callchain data as normal hist entries, while the --max-stack control the limit
of single entry callchain depth.

Using the callchain_cursor.nr as he_cache array count to fix this. Also
removing struct hist_entry_iter::max_stack, because there's no longer any use
for it.

We need more fixes to ensure that the branch stack code follows properly the
logic of --max-stack, which is not the case at the moment.
Original-patch-by: default avatarJin Yao <yao.jin@linux.intel.com>
Signed-off-by: default avatarJiri Olsa <jolsa@kernel.org>
Reported-by: default avatarJin Yao <yao.jin@linux.intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20180216123619.GA9945@kravaSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: default avatarSasha Levin <alexander.levin@microsoft.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: default avatarJuerg Haefliger <juergh@canonical.com>
Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
parent 257623ea
...@@ -720,7 +720,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter, ...@@ -720,7 +720,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
* cumulated only one time to prevent entries more than 100% * cumulated only one time to prevent entries more than 100%
* overhead. * overhead.
*/ */
he_cache = malloc(sizeof(*he_cache) * (iter->max_stack + 1)); he_cache = malloc(sizeof(*he_cache) * (callchain_cursor.nr + 1));
if (he_cache == NULL) if (he_cache == NULL)
return -ENOMEM; return -ENOMEM;
...@@ -881,8 +881,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, ...@@ -881,8 +881,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
if (err) if (err)
return err; return err;
iter->max_stack = max_stack_depth;
err = iter->ops->prepare_entry(iter, al); err = iter->ops->prepare_entry(iter, al);
if (err) if (err)
goto out; goto out;
......
...@@ -91,7 +91,6 @@ struct hist_entry_iter { ...@@ -91,7 +91,6 @@ struct hist_entry_iter {
int curr; int curr;
bool hide_unresolved; bool hide_unresolved;
int max_stack;
struct perf_evsel *evsel; struct perf_evsel *evsel;
struct perf_sample *sample; struct perf_sample *sample;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment