* [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
@ 2018-02-13 8:44 Jin Yao
2018-02-13 9:45 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Jin Yao @ 2018-02-13 8:44 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
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]
The memory corruption happens at:
iter_add_next_cumulative_entry()
{
...
for (i = 0; i < iter->curr; i++) {
...
}
Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(),
they all don't check if iter->curr exceeds the array 'he_cache[]'.
If there are too many nodes in callchain, it's possible that iter->curr >
iter->max_stack, then memory corruption occurs.
This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry()
if necessary (the case of too many nodes in callchain).
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/util/hist.c | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b614095..71f07d2 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -926,11 +926,32 @@ iter_next_cumulative_entry(struct hist_entry_iter *iter,
struct addr_location *al)
{
struct callchain_cursor_node *node;
+ struct hist_entry **tmp;
+ int i;
node = callchain_cursor_current(&callchain_cursor);
if (node == NULL)
return 0;
+ /*
+ * If there are too many nodes in callchain,
+ * increase the size of he_cache[].
+ */
+ if (iter->curr == iter->max_stack) {
+ i = 2 * iter->max_stack + 1;
+ tmp = realloc(iter->priv, sizeof(struct hist_entry *) * i);
+ if (tmp == NULL) {
+ /*
+ * No need to free iter->priv here. It will be
+ * freed in iter_finish_cumulative_entry.
+ */
+ return 0;
+ }
+
+ iter->priv = tmp;
+ iter->max_stack = i;
+ }
+
return fill_callchain_info(al, node, iter->hide_unresolved);
}
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
2018-02-13 8:44 [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history Jin Yao
@ 2018-02-13 9:45 ` Jiri Olsa
2018-02-13 14:00 ` Jin, Yao
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2018-02-13 9:45 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote:
> 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]
>
> The memory corruption happens at:
>
> iter_add_next_cumulative_entry()
> {
> ...
> for (i = 0; i < iter->curr; i++) {
> ...
> }
>
> Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(),
> they all don't check if iter->curr exceeds the array 'he_cache[]'.
>
> If there are too many nodes in callchain, it's possible that iter->curr >
> iter->max_stack, then memory corruption occurs.
>
> This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry()
> if necessary (the case of too many nodes in callchain).
right, the max_stack does not say how many nodes end up in
callchain_cursor at the end.. good catch, please mention
that also in the changelog
however we know the final count from callchain_cursor itself,
the attached patch might do the same job, right?
also could we now get rid of iter->max_stack?
thanks,
jirka
---
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b6140950301e..b50b7b70dcca 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
* cumulated only one time to prevent entries more than 100%
* 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)
return -ENOMEM;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
2018-02-13 9:45 ` Jiri Olsa
@ 2018-02-13 14:00 ` Jin, Yao
2018-02-16 2:25 ` Jin, Yao
0 siblings, 1 reply; 8+ messages in thread
From: Jin, Yao @ 2018-02-13 14:00 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 2/13/2018 5:45 PM, Jiri Olsa wrote:
> On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote:
>> 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]
>>
>> The memory corruption happens at:
>>
>> iter_add_next_cumulative_entry()
>> {
>> ...
>> for (i = 0; i < iter->curr; i++) {
>> ...
>> }
>>
>> Whatever in iter_next_cumulative_entry() or in iter_add_next_cumulative_entry(),
>> they all don't check if iter->curr exceeds the array 'he_cache[]'.
>>
>> If there are too many nodes in callchain, it's possible that iter->curr >
>> iter->max_stack, then memory corruption occurs.
>>
>> This patch will reallocate array 'he_cache[]' in iter_next_cumulative_entry()
>> if necessary (the case of too many nodes in callchain).
>
> right, the max_stack does not say how many nodes end up in
> callchain_cursor at the end.. good catch, please mention
> that also in the changelog
>
max_stack looks only to limit the number of calls but not for other
branches.
> however we know the final count from callchain_cursor itself,
> the attached patch might do the same job, right?
>
I think the attached patch is ok.
> also could we now get rid of iter->max_stack?
>
From my opinion, the option '--max-stack' in perf report looks not very
necessary. While it's just my personal opinion, need to hear from more
people. :)
Thanks
Jin Yao
> thanks,
> jirka
>
>
> ---
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index b6140950301e..b50b7b70dcca 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
> * cumulated only one time to prevent entries more than 100%
> * 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)
> return -ENOMEM;
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
2018-02-13 14:00 ` Jin, Yao
@ 2018-02-16 2:25 ` Jin, Yao
2018-02-16 7:53 ` Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Jin, Yao @ 2018-02-16 2:25 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 2/13/2018 10:00 PM, Jin, Yao wrote:
>
>
> On 2/13/2018 5:45 PM, Jiri Olsa wrote:
>> On Tue, Feb 13, 2018 at 04:44:28PM +0800, Jin Yao wrote:
>>> 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]
>>>
>>> The memory corruption happens at:
>>>
>>> iter_add_next_cumulative_entry()
>>> {
>>> ...
>>> for (i = 0; i < iter->curr; i++) {
>>> ...
>>> }
>>>
>>> Whatever in iter_next_cumulative_entry() or in
>>> iter_add_next_cumulative_entry(),
>>> they all don't check if iter->curr exceeds the array 'he_cache[]'.
>>>
>>> If there are too many nodes in callchain, it's possible that
>>> iter->curr >
>>> iter->max_stack, then memory corruption occurs.
>>>
>>> This patch will reallocate array 'he_cache[]' in
>>> iter_next_cumulative_entry()
>>> if necessary (the case of too many nodes in callchain).
>>
>> right, the max_stack does not say how many nodes end up in
>> callchain_cursor at the end.. good catch, please mention
>> that also in the changelog
>>
>
> max_stack looks only to limit the number of calls but not for other
> branches.
>
>> however we know the final count from callchain_cursor itself,
>> the attached patch might do the same job, right?
>>
>
> I think the attached patch is ok.
>
>> also could we now get rid of iter->max_stack?
>>
>
> From my opinion, the option '--max-stack' in perf report looks not very
> necessary. While it's just my personal opinion, need to hear from more
> people. :)
>
> Thanks
> Jin Yao
>
>> thanks,
>> jirka
>>
>>
>> ---
>> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
>> index b6140950301e..b50b7b70dcca 100644
>> --- a/tools/perf/util/hist.c
>> +++ b/tools/perf/util/hist.c
>> @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct
>> hist_entry_iter *iter,
>> * cumulated only one time to prevent entries more than 100%
>> * 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)
>> return -ENOMEM;
>>
Hi Jiri,
I guess you will post this patch, right?
Thanks
Jin Yao
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history
2018-02-16 2:25 ` Jin, Yao
@ 2018-02-16 7:53 ` Jiri Olsa
2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa
0 siblings, 1 reply; 8+ messages in thread
From: Jiri Olsa @ 2018-02-16 7:53 UTC (permalink / raw)
To: Jin, Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Fri, Feb 16, 2018 at 10:25:31AM +0800, Jin, Yao wrote:
SNIP
> > From my opinion, the option '--max-stack' in perf report looks not very
> > necessary. While it's just my personal opinion, need to hear from more
> > people. :)
> >
> > Thanks
> > Jin Yao
> >
> > > thanks,
> > > jirka
> > >
> > >
> > > ---
> > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > > index b6140950301e..b50b7b70dcca 100644
> > > --- a/tools/perf/util/hist.c
> > > +++ b/tools/perf/util/hist.c
> > > @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct
> > > hist_entry_iter *iter,
> > > * cumulated only one time to prevent entries more than 100%
> > > * 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)
> > > return -ENOMEM;
> > >
>
> Hi Jiri,
>
> I guess you will post this patch, right?
yep, later today
jirka
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history
2018-02-16 7:53 ` Jiri Olsa
@ 2018-02-16 12:36 ` Jiri Olsa
2018-02-16 13:02 ` Arnaldo Carvalho de Melo
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa
0 siblings, 2 replies; 8+ messages in thread
From: Jiri Olsa @ 2018-02-16 12:36 UTC (permalink / raw)
To: Jin, Yao, acme
Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Fri, Feb 16, 2018 at 08:53:04AM +0100, Jiri Olsa wrote:
> On Fri, Feb 16, 2018 at 10:25:31AM +0800, Jin, Yao wrote:
>
> SNIP
>
> > > From my opinion, the option '--max-stack' in perf report looks not very
> > > necessary. While it's just my personal opinion, need to hear from more
> > > people. :)
> > >
> > > Thanks
> > > Jin Yao
> > >
> > > > thanks,
> > > > jirka
> > > >
> > > >
> > > > ---
> > > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> > > > index b6140950301e..b50b7b70dcca 100644
> > > > --- a/tools/perf/util/hist.c
> > > > +++ b/tools/perf/util/hist.c
> > > > @@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct
> > > > hist_entry_iter *iter,
> > > > * cumulated only one time to prevent entries more than 100%
> > > > * 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)
> > > > return -ENOMEM;
> > > >
> >
> > Hi Jiri,
> >
> > I guess you will post this patch, right?
>
> yep, later today
here it is.. I think we want this change now to fix the crash, and
some more fixes later to ensure that the branch stack code follows
properly the logic of --max-stack, which is not the case now
thanks,
jirka
---
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 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.
Reported-by: Jin Yao <yao.jin@linux.intel.com>
Original-patch-by: Jin Yao <yao.jin@linux.intel.com>
Link: http://lkml.kernel.org/n/tip-qj1kdpvyu25ac6w22lhmy7m2@git.kernel.org
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
tools/perf/util/hist.c | 4 +---
tools/perf/util/hist.h | 1 -
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b6140950301e..44a8456cea10 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
* cumulated only one time to prevent entries more than 100%
* 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)
return -ENOMEM;
@@ -1045,8 +1045,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
if (err)
return err;
- iter->max_stack = max_stack_depth;
-
err = iter->ops->prepare_entry(iter, al);
if (err)
goto out;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 02721b579746..e869cad4d89f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -107,7 +107,6 @@ struct hist_entry_iter {
int curr;
bool hide_unresolved;
- int max_stack;
struct perf_evsel *evsel;
struct perf_sample *sample;
--
2.13.6
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history
2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa
@ 2018-02-16 13:02 ` Arnaldo Carvalho de Melo
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa
1 sibling, 0 replies; 8+ messages in thread
From: Arnaldo Carvalho de Melo @ 2018-02-16 13:02 UTC (permalink / raw)
To: Jiri Olsa
Cc: Jin, Yao, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
ak, kan.liang, yao.jin
Em Fri, Feb 16, 2018 at 01:36:19PM +0100, Jiri Olsa escreveu:
>
> here it is.. I think we want this change now to fix the crash, and
> some more fixes later to ensure that the branch stack code follows
> properly the logic of --max-stack, which is not the case now
Thanks, applied.
- Arnaldo
^ permalink raw reply [flat|nested] 8+ messages in thread
* [tip:perf/core] perf report: Fix memory corruption in --branch-history mode --branch-history
2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa
2018-02-16 13:02 ` Arnaldo Carvalho de Melo
@ 2018-02-17 11:34 ` tip-bot for Jiri Olsa
1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Jiri Olsa @ 2018-02-17 11:34 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, peterz, hpa, mingo, jolsa, alexander.shishkin, kan.liang,
tglx, ak, jolsa, yao.jin, linux-kernel
Commit-ID: e3ebaa465136ecfedf9c6f4671df02bf625f8125
Gitweb: https://git.kernel.org/tip/e3ebaa465136ecfedf9c6f4671df02bf625f8125
Author: Jiri Olsa <jolsa@redhat.com>
AuthorDate: Fri, 16 Feb 2018 13:36:19 +0100
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 16 Feb 2018 14:55:47 -0300
perf report: Fix memory corruption in --branch-history mode --branch-history
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: Jin Yao <yao.jin@linux.intel.com>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Reported-by: Jin 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@krava
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/hist.c | 4 +---
tools/perf/util/hist.h | 1 -
2 files changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b614095..44a8456 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -879,7 +879,7 @@ iter_prepare_cumulative_entry(struct hist_entry_iter *iter,
* cumulated only one time to prevent entries more than 100%
* 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)
return -ENOMEM;
@@ -1045,8 +1045,6 @@ int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al,
if (err)
return err;
- iter->max_stack = max_stack_depth;
-
err = iter->ops->prepare_entry(iter, al);
if (err)
goto out;
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 02721b57..e869cad 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -107,7 +107,6 @@ struct hist_entry_iter {
int curr;
bool hide_unresolved;
- int max_stack;
struct perf_evsel *evsel;
struct perf_sample *sample;
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-02-17 11:45 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-13 8:44 [PATCH] perf report: Fix a memory corrupton issue when enabling --branch-history Jin Yao
2018-02-13 9:45 ` Jiri Olsa
2018-02-13 14:00 ` Jin, Yao
2018-02-16 2:25 ` Jin, Yao
2018-02-16 7:53 ` Jiri Olsa
2018-02-16 12:36 ` [PATCH] perf report: Fix memory corruption in --branch-history mode --branch-history Jiri Olsa
2018-02-16 13:02 ` Arnaldo Carvalho de Melo
2018-02-17 11:34 ` [tip:perf/core] " tip-bot for Jiri Olsa
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox