Em Thu, Nov 10, 2016 at 04:40:46PM -0800, Krister Johansen escreveu:
> On Wed, Oct 26, 2016 at 11:44:53AM -0200, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 25, 2016 at 05:20:10PM -0700, Krister Johansen escreveu:
> > > On Tue, Oct 11, 2016 at 02:28:39AM -0700, Krister Johansen wrote:
> > > > If dso__load_kcore frees all of the existing maps, but one has already
> > > > been attached to a callchain cursor node, then we can get a SIGSEGV in
> > > > any function that happens to try to use this invalid cursor. Use the
> > > > existing map refcount mechanism to forestall cleanup of a map until the
> > > > cursor iterates past the node.
> > >
> > > It has been a couple of weeks since I sent out v2 of this patch. I
> > > understand that folks here have plenty of irons in the fire, but I
> > > wanted to double-check that nobody was waiting on me for additional
> > > information or changes.
> >
> > It was a mix of waiting for more people to review it, or for Masami to
> > run its refcount debugger on it, ended up falling thru the cracks.
> >
> > I'll try to process it now.
>
> Thanks. As part of processing this did you run into any problems?
> Would you like me to rebase against the latest perf/core and re-send the
> patch?
Sorry for the overly long delay, trying it now after fixing up a
conflict with a recent patchkit (branch stuff) I tested it by running
'perf top -g' and I'm getting some assertion bugs:
# perf top -g
1.34% filemap_map_pages
- 0.59% alloc_pages_vma
1.20% __alloc_pages_nodemask
- 5.87% 0.45% [kernel] [k] handle_mm_fault
- 1.94% handle_mm_fault
1.34% filemap_map_pages
- 0.59% alloc_pages_vma
1.22% __alloc_pages_nodemask
+ 5.75% 0.03% perf [.]
hist_entry_iter__add
+ 4.46% 0.00% [unknown] [.] 0000000000000000
- 4.06% 2.74% libc-2.23.so [.] _int_malloc
- 1.95% 0
1.94% _int_malloc
- 3.20% 0.23% perf [.]
iter_add_next_cumulative_entry
- 1.49% iter_add_next_cumulative_entry
- 1.43% __hists__add_entry
2.58% 0.01% [kernel] [k]
return_from_SYSCALL_64
2.57% 2.55% libperl.so.5.22.2 [.] Perl_fbm_instr
- 2.54% 2.51% liblzma.so.5.2.2 [.] lzma_decode
- 2.51% lzma_decode
2.33% 0.00% ld-2.23.so [.] _dl_sysdep_start
+ 2.24% 0.04% ld-2.23.so [.] dl_main
2.13% 0.03% [kernel] [k] ext4_readdir
2.09% 0.01% [kernel] [k] sys_newstat
2.08% 0.04% [kernel] [k] vfs_fstatat
2.07% 0.02% [kernel] [k] SYSC_newstat
2.02% 0.01% [kernel] [k] iterate_dir
- 1.96% 0.17% [kernel] [k]
__alloc_pages_nodemask
- 1.37% __alloc_pages_nodemask
perf: util/map.c:246: map__exit: Assertion
`!(!((&map->rb_node)->__rb_parent_color == (unsigned long)(&map->rb_node)))'
failed.
Aborted (core dumped)
[root@jouet ~]#
I'll try to investigate this further later/tomorrow, find the updated patch
below.
- Arnaldo
commit af04d2c4a5d1f6bd7f4971118e4e1153cc7c2506
Author: Krister Johansen <k...@templeofstupid.com>
Date: Tue Oct 11 02:28:39 2016 -0700
perf callchain: Fix a use after free crash due to refcounting bug
If dso__load_kcore frees all of the existing maps, but one has already
been attached to a callchain cursor node, then we can get a SIGSEGV in
any function that happens to try to use this invalid cursor. Use the
existing map refcount mechanism to forestall cleanup of a map until the
cursor iterates past the node.
Signed-off-by: Krister Johansen <k...@templeofstupid.com>
Cc: Frederic Weisbecker <fweis...@gmail.com>
Cc: Masami Hiramatsu <mhira...@kernel.org>
Cc: Namhyung Kim <namhy...@kernel.org>
Link: http://lkml.kernel.org/r/20161011092839.gc7...@templeofstupid.com
Signed-off-by: Arnaldo Carvalho de Melo <a...@redhat.com>
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 823befd8209a..18bb7caee535 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -437,7 +437,7 @@ fill_node(struct callchain_node *node, struct
callchain_cursor *cursor)
}
call->ip = cursor_node->ip;
call->ms.sym = cursor_node->sym;
- call->ms.map = cursor_node->map;
+ call->ms.map = map__get(cursor_node->map);
if (cursor_node->branch) {
call->branch_count = 1;
@@ -477,6 +477,7 @@ add_child(struct callchain_node *parent,
list_for_each_entry_safe(call, tmp, &new->val, list) {
list_del(&call->list);
+ map__zput(call->ms.map);
free(call);
}
free(new);
@@ -761,6 +762,7 @@ merge_chain_branch(struct callchain_cursor *cursor,
list->ms.map, list->ms.sym,
false, NULL, 0, 0);
list_del(&list->list);
+ map__zput(list->ms.map);
free(list);
}
@@ -811,7 +813,8 @@ int callchain_cursor_append(struct callchain_cursor *cursor,
}
node->ip = ip;
- node->map = map;
+ map__zput(node->map);
+ node->map = map__get(map);
node->sym = sym;
node->branch = branch;
node->nr_loop_iter = nr_loop_iter;
@@ -868,6 +871,8 @@ int fill_callchain_info(struct addr_location *al, struct
callchain_cursor_node *
goto out;
}
+ map__get(al->map);
+
if (al->map->groups == &al->machine->kmaps) {
if (machine__is_host(al->machine)) {
al->cpumode = PERF_RECORD_MISC_KERNEL;
@@ -1142,11 +1147,13 @@ static void free_callchain_node(struct callchain_node
*node)
list_for_each_entry_safe(list, tmp, &node->parent_val, list) {
list_del(&list->list);
+ map__zput(list->ms.map);
free(list);
}
list_for_each_entry_safe(list, tmp, &node->val, list) {
list_del(&list->list);
+ map__zput(list->ms.map);
free(list);
}
@@ -1210,6 +1217,7 @@ int callchain_node__make_parent_list(struct
callchain_node *node)
goto out;
*new = *chain;
new->has_children = false;
+ map__get(new->ms.map);
list_add_tail(&new->list, &head);
}
parent = parent->parent;
@@ -1230,6 +1238,7 @@ int callchain_node__make_parent_list(struct
callchain_node *node)
out:
list_for_each_entry_safe(chain, new, &head, list) {
list_del(&chain->list);
+ map__zput(chain->ms.map);
free(chain);
}
return -ENOMEM;
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index d9c70dccf06a..f551fd2cfe5a 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -5,6 +5,7 @@
#include <linux/list.h>
#include <linux/rbtree.h>
#include "event.h"
+#include "map.h"
#include "symbol.h"
#define HELP_PAD "\t\t\t\t"
@@ -184,8 +185,13 @@ int callchain_merge(struct callchain_cursor *cursor,
*/
static inline void callchain_cursor_reset(struct callchain_cursor *cursor)
{
+ struct callchain_cursor_node *node;
+
cursor->nr = 0;
cursor->last = &cursor->first;
+
+ for (node = cursor->first; node != NULL; node = node->next)
+ map__zput(node->map);
}
int callchain_cursor_append(struct callchain_cursor *cursor, u64 ip,
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index e1be4132054d..be4b07145705 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;
}
