From: Jiri Olsa <jolsa@redhat.com>
To: Changbin Du <changbin.du@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
Jiri Olsa <jolsa@kernel.org>,
namhyung@kernel.org, Ingo Molnar <mingo@redhat.com>,
Peter Zijlstra <peterz@infradead.org>,
Alexei Starovoitov <ast@kernel.org>,
rostedt@goodmis.org, Daniel Borkmann <daniel@iogearbox.net>,
bpf@vger.kernel.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Subject: Re: [PATCH 07/16] perf: top: fix heap-use-after-free issue
Date: Mon, 18 Mar 2019 11:08:05 +0100 [thread overview]
Message-ID: <20190318100805.GB28556@krava> (raw)
In-Reply-To: <20190316080556.3075-8-changbin.du@gmail.com>
On Sat, Mar 16, 2019 at 04:05:47PM +0800, Changbin Du wrote:
> The evlist should be destroyed before the perf session.
>
> =================================================================
> ==27350==ERROR: AddressSanitizer: heap-use-after-free on address 0x62b000002e38 at pc 0x5611da276999 bp 0x7ffce8f1d1a0 sp 0x7ffce8f1d190
> WRITE of size 8 at 0x62b000002e38 thread T0
> #0 0x5611da276998 in __list_del /home/work/linux/tools/include/linux/list.h:89
> #1 0x5611da276d4a in __list_del_entry /home/work/linux/tools/include/linux/list.h:102
> #2 0x5611da276e77 in list_del_init /home/work/linux/tools/include/linux/list.h:145
> #3 0x5611da2781cd in thread__put util/thread.c:130
ah, because thread is on the machine's list.. which is inside session
looks like we could use refference counts in machine in future
thanks,
jirka
> #4 0x5611da2cc0a8 in __thread__zput util/thread.h:68
> #5 0x5611da2d2dcb in hist_entry__delete util/hist.c:1148
> #6 0x5611da2cdf91 in hists__delete_entry util/hist.c:337
> #7 0x5611da2ce19e in hists__delete_entries util/hist.c:365
> #8 0x5611da2db2ab in hists__delete_all_entries util/hist.c:2639
> #9 0x5611da2db325 in hists_evsel__exit util/hist.c:2651
> #10 0x5611da1c5352 in perf_evsel__exit util/evsel.c:1304
> #11 0x5611da1c5390 in perf_evsel__delete util/evsel.c:1309
> #12 0x5611da1b35f0 in perf_evlist__purge util/evlist.c:124
> #13 0x5611da1b38e2 in perf_evlist__delete util/evlist.c:148
> #14 0x5611da069781 in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1645
> #15 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
> #16 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
> #17 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
> #18 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
> #19 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
> #20 0x5611d9ff35c9 in _start (/home/work/linux/tools/perf/perf+0x3e95c9)
>
> 0x62b000002e38 is located 11320 bytes inside of 27448-byte region [0x62b000000200,0x62b000006d38)
> freed by thread T0 here:
> #0 0x7fdccb04ab70 in free (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xedb70)
> #1 0x5611da260df4 in perf_session__delete util/session.c:201
> #2 0x5611da063de5 in __cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1300
> #3 0x5611da06973c in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1642
> #4 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
> #5 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
> #6 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
> #7 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
> #8 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
>
> previously allocated by thread T0 here:
> #0 0x7fdccb04b138 in calloc (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xee138)
> #1 0x5611da26010c in zalloc util/util.h:23
> #2 0x5611da260824 in perf_session__new util/session.c:118
> #3 0x5611da0633a6 in __cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1192
> #4 0x5611da06973c in cmd_top /home/changbin/work/linux/tools/perf/builtin-top.c:1642
> #5 0x5611da17d038 in run_builtin /home/changbin/work/linux/tools/perf/perf.c:302
> #6 0x5611da17d577 in handle_internal_command /home/changbin/work/linux/tools/perf/perf.c:354
> #7 0x5611da17d97b in run_argv /home/changbin/work/linux/tools/perf/perf.c:398
> #8 0x5611da17e0e9 in main /home/changbin/work/linux/tools/perf/perf.c:520
> #9 0x7fdcc970f09a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2409a)
>
> SUMMARY: AddressSanitizer: heap-use-after-free /home/work/linux/tools/include/linux/list.h:89 in __list_del
> Shadow bytes around the buggy address:
> 0x0c567fff8570: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff8580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff8590: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff85a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff85b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> =>0x0c567fff85c0: fd fd fd fd fd fd fd[fd]fd fd fd fd fd fd fd fd
> 0x0c567fff85d0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff85e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff85f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff8600: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> 0x0c567fff8610: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
> Shadow byte legend (one shadow byte represents 8 application bytes):
> Addressable: 00
> Partially addressable: 01 02 03 04 05 06 07
> Heap left redzone: fa
> Freed heap region: fd
> Stack left redzone: f1
> Stack mid redzone: f2
> Stack right redzone: f3
> Stack after return: f5
> Stack use after scope: f8
> Global redzone: f9
> Global init order: f6
> Poisoned by user: f7
> Container overflow: fc
> Array cookie: ac
> Intra object redzone: bb
> ASan internal: fe
> Left alloca redzone: ca
> Right alloca redzone: cb
> ==27350==ABORTING
>
> Signed-off-by: Changbin Du <changbin.du@gmail.com>
> ---
> tools/perf/builtin-top.c | 42 ++++++++++++++++++----------------------
> 1 file changed, 19 insertions(+), 23 deletions(-)
>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 231a90daa958..614f278235fa 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1189,23 +1189,19 @@ static int __cmd_top(struct perf_top *top)
> pthread_t thread, thread_process;
> int ret;
>
> - top->session = perf_session__new(NULL, false, NULL);
> - if (top->session == NULL)
> - return -1;
> -
> if (!top->annotation_opts.objdump_path) {
> ret = perf_env__lookup_objdump(&top->session->header.env,
> &top->annotation_opts.objdump_path);
> if (ret)
> - goto out_delete;
> + return ret;
> }
>
> ret = callchain_param__setup_sample_type(&callchain_param);
> if (ret)
> - goto out_delete;
> + return ret;
>
> if (perf_session__register_idle_thread(top->session) < 0)
> - goto out_delete;
> + return ret;
>
> if (top->nr_threads_synthesize > 1)
> perf_set_multithreaded();
> @@ -1227,13 +1223,18 @@ static int __cmd_top(struct perf_top *top)
>
> if (perf_hpp_list.socket) {
> ret = perf_env__read_cpu_topology_map(&perf_env);
> - if (ret < 0)
> - goto out_err_cpu_topo;
> + if (ret < 0) {
> + char errbuf[BUFSIZ];
> + const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
> +
> + ui__error("Could not read the CPU topology map: %s\n", err);
> + return ret;
> + }
> }
>
> ret = perf_top__start_counters(top);
> if (ret)
> - goto out_delete;
> + return ret;
>
> top->session->evlist = top->evlist;
> perf_session__set_id_hdr_size(top->session);
> @@ -1252,7 +1253,7 @@ static int __cmd_top(struct perf_top *top)
> ret = -1;
> if (pthread_create(&thread_process, NULL, process_thread, top)) {
> ui__error("Could not create process thread.\n");
> - goto out_delete;
> + return ret;
> }
>
> if (pthread_create(&thread, NULL, (use_browser > 0 ? display_thread_tui :
> @@ -1296,19 +1297,7 @@ static int __cmd_top(struct perf_top *top)
> out_join_thread:
> pthread_cond_signal(&top->qe.cond);
> pthread_join(thread_process, NULL);
> -out_delete:
> - perf_session__delete(top->session);
> - top->session = NULL;
> -
> return ret;
> -
> -out_err_cpu_topo: {
> - char errbuf[BUFSIZ];
> - const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
> -
> - ui__error("Could not read the CPU topology map: %s\n", err);
> - goto out_delete;
> -}
> }
>
> static int
> @@ -1639,10 +1628,17 @@ int cmd_top(int argc, const char **argv)
> signal(SIGWINCH, winch_sig);
> }
>
> + top.session = perf_session__new(NULL, false, NULL);
> + if (top.session == NULL) {
> + status = -1;
> + goto out_delete_evlist;
> + }
> +
> status = __cmd_top(&top);
>
> out_delete_evlist:
> perf_evlist__delete(top.evlist);
> + perf_session__delete(top.session);
>
> return status;
> }
> --
> 2.19.1
>
next prev parent reply other threads:[~2019-03-18 10:08 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-16 8:05 [PATCH 00/16] fix some perf issues detected by ASan Changbin Du
2019-03-16 8:05 ` [PATCH 01/16] perf: add doc for how to build perf with Asan and UBSan Changbin Du
2019-03-18 10:07 ` Jiri Olsa
2019-03-18 20:08 ` Arnaldo Carvalho de Melo
2019-03-20 12:00 ` Changbin Du
2019-03-20 11:58 ` Changbin Du
2019-03-16 8:05 ` [PATCH 02/16] perf: list: fix memory leak in function is_event_supported Changbin Du
2019-03-16 8:05 ` [PATCH 03/16] perf: fix errors under optimization level '-Og' Changbin Du
2019-03-16 8:05 ` [PATCH 04/16] perf: fix an error in config template Changbin Du
2019-03-16 8:05 ` [PATCH 05/16] perf: fix a memory leak in collect_config Changbin Du
2019-03-16 8:05 ` [PATCH 06/16] perf: fix memory leak in print_sdt_events() Changbin Du
2019-03-16 8:05 ` [PATCH 07/16] perf: top: fix heap-use-after-free issue Changbin Du
2019-03-18 10:08 ` Jiri Olsa [this message]
2019-03-16 8:05 ` [PATCH 08/16] perf: top: fix error handing in cmd_top() Changbin Du
2019-03-16 8:05 ` [PATCH 09/16] perf: missed a map__put() in error case Changbin Du
2019-03-16 8:05 ` [PATCH 10/16] perf: remove map from names tree in __maps__remove Changbin Du
2019-03-16 8:05 ` [PATCH 11/16] perf: purge all maps from the names tree Changbin Du
2019-03-16 8:05 ` [PATCH 12/16] perf: top: fix global-buffer-overflow issue Changbin Du
2019-03-16 8:05 ` [PATCH 13/16] perf: free all counts in perf_evsel__exit Changbin Du
2019-03-18 19:39 ` Arnaldo Carvalho de Melo
2019-03-16 8:05 ` [PATCH 14/16] perf: fix a memory leak of cpu_map object Changbin Du
2019-03-16 8:05 ` [PATCH 15/16] perf: fix memory leak by expr__find_other Changbin Du
2019-03-16 8:05 ` [PATCH 16/16] perf: fix a memory leak in test__perf_evsel__tp_sched_test Changbin Du
2019-03-18 10:08 ` [PATCH 00/16] fix some perf issues detected by ASan Jiri Olsa
2019-03-18 16:16 ` Arnaldo Carvalho de Melo
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=20190318100805.GB28556@krava \
--to=jolsa@redhat.com \
--cc=acme@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=changbin.du@gmail.com \
--cc=daniel@iogearbox.net \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=namhyung@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.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).