All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Krister Johansen <kjlx@templeofstupid.com>
Cc: "Namhyung Kim" <namhyung@kernel.org>,
	"Masami Hiramatsu" <mhiramat@kernel.org>,
	"Frédéric Weisbecker" <fweisbec@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 perf/core] perf script: fix a use after free crash.
Date: Mon, 2 Jan 2017 14:36:57 -0300	[thread overview]
Message-ID: <20170102173657.GB27864@kernel.org> (raw)
In-Reply-To: <20170102173530.GA27864@kernel.org>

Em Mon, Jan 02, 2017 at 02:35:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Jan 02, 2017 at 12:15:14PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Tried it again with what is in Linus' tree + your patch and got the same
> > problem:
> > 
> > [acme@jouet linux]$ git remote -v | grep torvalds.*fetch
> > torvalds	git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git (fetch)
> > [acme@jouet linux]$ git checkout -b test-branch torvalds/master
> > Branch test-branch set up to track remote branch master from torvalds.
> > Switched to a new branch 'test-branch'
> > [acme@jouet linux]$ git cherry-pick f7347a33099dbad7e9fb3c22cea211f238bfd320
> > [test-branch 7d786f548b62] perf callchain: Fix a use after free crash due to refcounting bug
> >  Author: Krister Johansen <kjlx@templeofstupid.com>
> >  Date: Mon Jan 2 12:06:55 2017 -0300
> >  3 files changed, 19 insertions(+), 2 deletions(-)
> > [acme@jouet linux]$ rm -rf /tmp/build/perf/ ; mkdir -p /tmp/build/perf ; make O=/tmp/build/perf -C tools/perf install-bin
> > make: Entering directory '/home/acme/git/linux/tools/perf'
> >   BUILD:   Doing 'make -j4' parallel build
> >   HOSTCC   /tmp/build/perf/fixdep.o
> > <SNIP>
> > 
> > Then I run it with a higher frequency and no delay in refreshing the screen, to
> > stress the refcounting code:
> > 
> > # perf top -F 10000 -g -d 0
> > 
> > Do it while running something like 'make -j32 allmodconfig' to create lots of
> > short lived processes (or use stress-ng, etc).
> 
> Back to acme/perf/core and with debugging, we're getting a refcount hitting zero
> while the map is still in a rbtree:

And this was introduced by your patch:

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6770a9645609..72f5c82798e9 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1,6 +1,7 @@
 #include "util.h"
 #include "build-id.h"
 #include "hist.h"
+#include "map.h"
 #include "session.h"
 #include "sort.h"
 #include "evlist.h"
@@ -979,6 +980,7 @@ iter_finish_cumulative_entry(struct hist_entry_iter *iter,
 {
        zfree(&iter->priv);
        iter->he = NULL;
+       map__zput(al->map);
 
        return 0;
 }
 
> perf: util/map.c:246: map__exit: Assertion `!(!((&map->rb_node)->__rb_parent_color == (unsigned long)(&map->rb_node)))' failed.
> 
>                                                                                                                                Thread 1 "perf" received signal SIGABRT, Aborted.
>                   0x00007ffff522691f in raise () from /lib64/libc.so.6
> (gdb) bt
> #0  0x00007ffff522691f in raise () from /lib64/libc.so.6
> #1  0x00007ffff522851a in abort () from /lib64/libc.so.6
> #2  0x00007ffff521eda7 in __assert_fail_base () from /lib64/libc.so.6
> #3  0x00007ffff521ee52 in __assert_fail () from /lib64/libc.so.6
> #4  0x0000000000504e57 in map__exit (map=0x2393790) at util/map.c:246
> #5  0x0000000000504ea5 in map__delete (map=0x2393790) at util/map.c:252
> #6  0x0000000000504f0a in map__put (map=0x2393790) at util/map.c:259
> #7  0x000000000052fa01 in __map__zput (map=0x7fffffff8230) at util/map.h:161
> #8  0x000000000053295b in iter_finish_cumulative_entry (iter=0x7fffffff8260, al=0x7fffffff8220) at util/hist.c:983
> #9  0x0000000000532b53 in hist_entry_iter__add (iter=0x7fffffff8260, al=0x7fffffff8220, max_stack_depth=127, arg=0x7fffffffa7b0) at util/hist.c:1059
> #10 0x000000000044f5cf in perf_event__process_sample (tool=0x7fffffffa7b0, event=0x7ffff7e24578, evsel=0x21515d0, sample=0x7fffffff8410, machine=0x21b2bf8)
>     at builtin-top.c:774
> #11 0x000000000044f8ee in perf_top__mmap_read_idx (top=0x7fffffffa7b0, idx=2) at builtin-top.c:840
> #12 0x000000000044fa0d in perf_top__mmap_read (top=0x7fffffffa7b0) at builtin-top.c:857
> #13 0x0000000000450080 in __cmd_top (top=0x7fffffffa7b0) at builtin-top.c:1002
> #14 0x00000000004514e0 in cmd_top (argc=0, argv=0x7fffffffe130, prefix=0x0) at builtin-top.c:1330
> #15 0x00000000004b5af5 in run_builtin (p=0xa0baf8 <commands+312>, argc=6, argv=0x7fffffffe130) at perf.c:358
> #16 0x00000000004b5d62 in handle_internal_command (argc=6, argv=0x7fffffffe130) at perf.c:420
> #17 0x00000000004b5ea7 in run_argv (argcp=0x7fffffffdf8c, argv=0x7fffffffdf80) at perf.c:466
> #18 0x00000000004b6290 in main (argc=6, argv=0x7fffffffe130) at perf.c:610
> (gdb) fr 4
> #4  0x0000000000504e57 in map__exit (map=0x2393790) at util/map.c:246
> 246		BUG_ON(!RB_EMPTY_NODE(&map->rb_node));
> (gdb) p map
> $1 = (struct map *) 0x2393790
> (gdb) p *map
> $2 = {{rb_node = {__rb_parent_color = 37304353, rb_right = 0x0, rb_left = 0x0}, node = {next = 0x2393821, prev = 0x0}}, start = 140434683187200, 
>   end = 140434690723840, type = 0 '\000', erange_warned = false, priv = 0, prot = 5, flags = 2, pgoff = 0, reloc = 0, maj = 253, min = 0, ino = 132875, 
>   ino_generation = 3472328296227680304, map_ip = 0x504125 <map__map_ip>, unmap_ip = 0x504174 <map__unmap_ip>, dso = 0x22b3890, groups = 0x2385290, refcnt = {
>     counter = 0}}
> (gdb)

  reply	other threads:[~2017-01-02 17:37 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-02  3:13 [PATCH perf/core] perf script: fix a use after free crash Krister Johansen
2016-10-05 11:45 ` callchain map refcounting fixes was " Arnaldo Carvalho de Melo
2016-10-06  0:29   ` Masami Hiramatsu
2016-10-06  6:12   ` Krister Johansen
2016-10-07  2:22   ` Namhyung Kim
2016-10-09  6:13     ` Krister Johansen
2016-10-11  9:28       ` Krister Johansen
2016-10-11  9:28     ` [PATCH v2 " Krister Johansen
2016-10-26  0:20       ` Krister Johansen
2016-10-26 13:44         ` Arnaldo Carvalho de Melo
2016-11-11  0:40           ` Krister Johansen
2016-11-22 19:01             ` Arnaldo Carvalho de Melo
2016-12-02  7:12               ` Krister Johansen
2016-12-29  1:39               ` Krister Johansen
2017-01-02 15:15                 ` Arnaldo Carvalho de Melo
2017-01-02 17:35                   ` Arnaldo Carvalho de Melo
2017-01-02 17:36                     ` Arnaldo Carvalho de Melo [this message]
2017-01-02 19:39                       ` Arnaldo Carvalho de Melo
2017-01-03  0:30                         ` Arnaldo Carvalho de Melo
2017-01-04  8:37                           ` Krister Johansen
2017-01-06  6:22                             ` Krister Johansen
2017-01-06  6:23                           ` [PATCH v3 " Krister Johansen
2017-01-21  1:48                             ` Krister Johansen
2017-02-01 14:39                             ` [tip:perf/core] perf callchain: Reference count maps tip-bot for Krister Johansen
2017-02-03 19:47                             ` [tip:perf/urgent] " tip-bot for Krister Johansen

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=20170102173657.GB27864@kernel.org \
    --to=acme@kernel.org \
    --cc=fweisbec@gmail.com \
    --cc=kjlx@templeofstupid.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=namhyung@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.