Skip to content

Commit 0dba9e4

Browse files
changbinduacmel
authored andcommitted
perf top: Delete the evlist before perf_session, fixing heap-use-after-free issue
The evlist should be destroyed before the perf session. Detected with gcc's ASan: ================================================================= ==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 #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 <[email protected]> Reviewed-by: Jiri Olsa <[email protected]> Cc: Alexei Starovoitov <[email protected]> Cc: Daniel Borkmann <[email protected]> Cc: Namhyung Kim <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Steven Rostedt (VMware) <[email protected]> Link: http://lkml.kernel.org/r/[email protected] Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
1 parent 8bde851 commit 0dba9e4

File tree

1 file changed

+19
-23
lines changed

1 file changed

+19
-23
lines changed

tools/perf/builtin-top.c

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,23 +1189,19 @@ static int __cmd_top(struct perf_top *top)
11891189
pthread_t thread, thread_process;
11901190
int ret;
11911191

1192-
top->session = perf_session__new(NULL, false, NULL);
1193-
if (top->session == NULL)
1194-
return -1;
1195-
11961192
if (!top->annotation_opts.objdump_path) {
11971193
ret = perf_env__lookup_objdump(&top->session->header.env,
11981194
&top->annotation_opts.objdump_path);
11991195
if (ret)
1200-
goto out_delete;
1196+
return ret;
12011197
}
12021198

12031199
ret = callchain_param__setup_sample_type(&callchain_param);
12041200
if (ret)
1205-
goto out_delete;
1201+
return ret;
12061202

12071203
if (perf_session__register_idle_thread(top->session) < 0)
1208-
goto out_delete;
1204+
return ret;
12091205

12101206
if (top->nr_threads_synthesize > 1)
12111207
perf_set_multithreaded();
@@ -1227,13 +1223,18 @@ static int __cmd_top(struct perf_top *top)
12271223

12281224
if (perf_hpp_list.socket) {
12291225
ret = perf_env__read_cpu_topology_map(&perf_env);
1230-
if (ret < 0)
1231-
goto out_err_cpu_topo;
1226+
if (ret < 0) {
1227+
char errbuf[BUFSIZ];
1228+
const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
1229+
1230+
ui__error("Could not read the CPU topology map: %s\n", err);
1231+
return ret;
1232+
}
12321233
}
12331234

12341235
ret = perf_top__start_counters(top);
12351236
if (ret)
1236-
goto out_delete;
1237+
return ret;
12371238

12381239
top->session->evlist = top->evlist;
12391240
perf_session__set_id_hdr_size(top->session);
@@ -1252,7 +1253,7 @@ static int __cmd_top(struct perf_top *top)
12521253
ret = -1;
12531254
if (pthread_create(&thread_process, NULL, process_thread, top)) {
12541255
ui__error("Could not create process thread.\n");
1255-
goto out_delete;
1256+
return ret;
12561257
}
12571258

12581259
if (pthread_create(&thread, NULL, (use_browser > 0 ? display_thread_tui :
@@ -1296,19 +1297,7 @@ static int __cmd_top(struct perf_top *top)
12961297
out_join_thread:
12971298
pthread_cond_signal(&top->qe.cond);
12981299
pthread_join(thread_process, NULL);
1299-
out_delete:
1300-
perf_session__delete(top->session);
1301-
top->session = NULL;
1302-
13031300
return ret;
1304-
1305-
out_err_cpu_topo: {
1306-
char errbuf[BUFSIZ];
1307-
const char *err = str_error_r(-ret, errbuf, sizeof(errbuf));
1308-
1309-
ui__error("Could not read the CPU topology map: %s\n", err);
1310-
goto out_delete;
1311-
}
13121301
}
13131302

13141303
static int
@@ -1639,10 +1628,17 @@ int cmd_top(int argc, const char **argv)
16391628
signal(SIGWINCH, winch_sig);
16401629
}
16411630

1631+
top.session = perf_session__new(NULL, false, NULL);
1632+
if (top.session == NULL) {
1633+
status = -1;
1634+
goto out_delete_evlist;
1635+
}
1636+
16421637
status = __cmd_top(&top);
16431638

16441639
out_delete_evlist:
16451640
perf_evlist__delete(top.evlist);
1641+
perf_session__delete(top.session);
16461642

16471643
return status;
16481644
}

0 commit comments

Comments
 (0)