| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20161229013947.GA2341@templeofstupid.com>
Date: Wed, 28 Dec 2016 17:39:47 -0800
From: Krister Johansen <kjlx@...pleofstupid.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Namhyung Kim <namhyung@...nel.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Frédéric Weisbecker <fweisbec@...il.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 perf/core] perf script: fix a use after free crash.
On Tue, Nov 22, 2016 at 04:01:06PM -0300, Arnaldo Carvalho de Melo wrote:
> 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@...et ~]#
>
>
> I'll try to investigate this further later/tomorrow, find the updated patch below.
>
> - Arnaldo
>
> commit af04d2c4a5d1f6bd7f4971118e4e1153cc7c2506
> Author: Krister Johansen <kjlx@...pleofstupid.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 <kjlx@...pleofstupid.com>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Masami Hiramatsu <mhiramat@...nel.org>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Link: http://lkml.kernel.org/r/20161011092839.GC7837@templeofstupid.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.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;
> }
As part of trying to tie up the year-end loose-ends, I went back and
re-tested a rebase'd version of this patch against perf/core. I ended
up with a merge that's identical to yours, except that I'm not seeing
any assertion failures with 'perf top -g', 'perf script', or 'perf
report'. Was perf/core the branch that was giving you trouble?
-K
Powered by blists - more mailing lists