linux-kernel.vger.kernel.org archive mirror
 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 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).