lkml.org 
[lkml]   [2018]   [May]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 321/329] perf report: Fix memory corruption in --branch-history mode --branch-history
    Date
    4.9-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Jiri Olsa <jolsa@redhat.com>

    [ Upstream commit e3ebaa465136ecfedf9c6f4671df02bf625f8125 ]

    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>
    Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    tools/perf/util/hist.c | 4 +---
    tools/perf/util/hist.h | 1 -
    2 files changed, 1 insertion(+), 4 deletions(-)

    --- a/tools/perf/util/hist.c
    +++ b/tools/perf/util/hist.c
    @@ -865,7 +865,7 @@ iter_prepare_cumulative_entry(struct his
    * 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;

    @@ -1030,8 +1030,6 @@ int hist_entry_iter__add(struct hist_ent
    if (err)
    return err;

    - iter->max_stack = max_stack_depth;
    -
    err = iter->ops->prepare_entry(iter, al);
    if (err)
    goto out;
    --- a/tools/perf/util/hist.h
    +++ b/tools/perf/util/hist.h
    @@ -102,7 +102,6 @@ struct hist_entry_iter {
    int curr;

    bool hide_unresolved;
    - int max_stack;

    struct perf_evsel *evsel;
    struct perf_sample *sample;

    \
     
     \ /
      Last update: 2018-05-28 15:54    [W:2.154 / U:0.084 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site