| Message ID | 20240301074639.2260708-1-irogers@google.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [v1,1/4] perf record: Delete session after stopping sideband thread | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <irogers@google.com> wrote: > > The session has a header in it which contains a perf env with > bpf_progs. The bpf_progs are accessed by the sideband thread and so > the sideband thread must be stopped before the session is deleted, to > avoid a use after free. This error was detected by AddressSanitizer > in the following: > > ``` > ==2054673==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000161e00 at pc 0x55769289de54 bp 0x7f9df36d4ab0 sp 0x7f9df36d4aa8 > READ of size 8 at 0x61d000161e00 thread T1 > #0 0x55769289de53 in __perf_env__insert_bpf_prog_info util/env.c:42 > #1 0x55769289dbb1 in perf_env__insert_bpf_prog_info util/env.c:29 > #2 0x557692bbae29 in perf_env__add_bpf_info util/bpf-event.c:483 > #3 0x557692bbb01a in bpf_event__sb_cb util/bpf-event.c:512 > #4 0x5576928b75f4 in perf_evlist__poll_thread util/sideband_evlist.c:68 > #5 0x7f9df96a63eb in start_thread nptl/pthread_create.c:444 > #6 0x7f9df9726a4b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 > > 0x61d000161e00 is located 384 bytes inside of 2136-byte region [0x61d000161c80,0x61d0001624d8) > freed by thread T0 here: > #0 0x7f9dfa6d7288 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52 > #1 0x557692978d50 in perf_session__delete util/session.c:319 > #2 0x557692673959 in __cmd_record tools/perf/builtin-record.c:2884 > #3 0x55769267a9f0 in cmd_record tools/perf/builtin-record.c:4259 > #4 0x55769286710c in run_builtin tools/perf/perf.c:349 > #5 0x557692867678 in handle_internal_command tools/perf/perf.c:402 > #6 0x557692867a40 in run_argv tools/perf/perf.c:446 > #7 0x557692867fae in main tools/perf/perf.c:562 > #8 0x7f9df96456c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > ``` > > Fixes: 657ee5531903 ("perf evlist: Introduce side band thread") > Signed-off-by: Ian Rogers <irogers@google.com> Note, after this series I'm seeing parallel perf testing being as reliable as serial but parallel testing is nearly 3 times faster. I think after these changes land we can make parallel execution the default. Thanks, Ian
On Thu, Feb 29, 2024 at 11:50 PM Ian Rogers <irogers@google.com> wrote: > > On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <irogers@google.com> wrote: > > > > The session has a header in it which contains a perf env with > > bpf_progs. The bpf_progs are accessed by the sideband thread and so > > the sideband thread must be stopped before the session is deleted, to > > avoid a use after free. This error was detected by AddressSanitizer > > in the following: > > > > ``` > > ==2054673==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000161e00 at pc 0x55769289de54 bp 0x7f9df36d4ab0 sp 0x7f9df36d4aa8 > > READ of size 8 at 0x61d000161e00 thread T1 > > #0 0x55769289de53 in __perf_env__insert_bpf_prog_info util/env.c:42 > > #1 0x55769289dbb1 in perf_env__insert_bpf_prog_info util/env.c:29 > > #2 0x557692bbae29 in perf_env__add_bpf_info util/bpf-event.c:483 > > #3 0x557692bbb01a in bpf_event__sb_cb util/bpf-event.c:512 > > #4 0x5576928b75f4 in perf_evlist__poll_thread util/sideband_evlist.c:68 > > #5 0x7f9df96a63eb in start_thread nptl/pthread_create.c:444 > > #6 0x7f9df9726a4b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 > > > > 0x61d000161e00 is located 384 bytes inside of 2136-byte region [0x61d000161c80,0x61d0001624d8) > > freed by thread T0 here: > > #0 0x7f9dfa6d7288 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52 > > #1 0x557692978d50 in perf_session__delete util/session.c:319 > > #2 0x557692673959 in __cmd_record tools/perf/builtin-record.c:2884 > > #3 0x55769267a9f0 in cmd_record tools/perf/builtin-record.c:4259 > > #4 0x55769286710c in run_builtin tools/perf/perf.c:349 > > #5 0x557692867678 in handle_internal_command tools/perf/perf.c:402 > > #6 0x557692867a40 in run_argv tools/perf/perf.c:446 > > #7 0x557692867fae in main tools/perf/perf.c:562 > > #8 0x7f9df96456c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > ``` > > > > Fixes: 657ee5531903 ("perf evlist: Introduce side band thread") > > Signed-off-by: Ian Rogers <irogers@google.com> > > Note, after this series I'm seeing parallel perf testing being as > reliable as serial but parallel testing is nearly 3 times faster. I > think after these changes land we can make parallel execution the > default. Ping. Thanks, Ian
On Tue, Mar 19, 2024 at 09:42:26PM -0700, Ian Rogers wrote: > On Thu, Feb 29, 2024 at 11:50 PM Ian Rogers <irogers@google.com> wrote: > > > > On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <irogers@google.com> wrote: > > > > > > The session has a header in it which contains a perf env with > > > bpf_progs. The bpf_progs are accessed by the sideband thread and so > > > the sideband thread must be stopped before the session is deleted, to > > > avoid a use after free. This error was detected by AddressSanitizer > > > in the following: > > > > > > ``` > > > ==2054673==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000161e00 at pc 0x55769289de54 bp 0x7f9df36d4ab0 sp 0x7f9df36d4aa8 > > > READ of size 8 at 0x61d000161e00 thread T1 > > > #0 0x55769289de53 in __perf_env__insert_bpf_prog_info util/env.c:42 > > > #1 0x55769289dbb1 in perf_env__insert_bpf_prog_info util/env.c:29 > > > #2 0x557692bbae29 in perf_env__add_bpf_info util/bpf-event.c:483 > > > #3 0x557692bbb01a in bpf_event__sb_cb util/bpf-event.c:512 > > > #4 0x5576928b75f4 in perf_evlist__poll_thread util/sideband_evlist.c:68 > > > #5 0x7f9df96a63eb in start_thread nptl/pthread_create.c:444 > > > #6 0x7f9df9726a4b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 > > > > > > 0x61d000161e00 is located 384 bytes inside of 2136-byte region [0x61d000161c80,0x61d0001624d8) > > > freed by thread T0 here: > > > #0 0x7f9dfa6d7288 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52 > > > #1 0x557692978d50 in perf_session__delete util/session.c:319 > > > #2 0x557692673959 in __cmd_record tools/perf/builtin-record.c:2884 > > > #3 0x55769267a9f0 in cmd_record tools/perf/builtin-record.c:4259 > > > #4 0x55769286710c in run_builtin tools/perf/perf.c:349 > > > #5 0x557692867678 in handle_internal_command tools/perf/perf.c:402 > > > #6 0x557692867a40 in run_argv tools/perf/perf.c:446 > > > #7 0x557692867fae in main tools/perf/perf.c:562 > > > #8 0x7f9df96456c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 > > > ``` > > > > > > Fixes: 657ee5531903 ("perf evlist: Introduce side band thread") > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > > Note, after this series I'm seeing parallel perf testing being as > > reliable as serial but parallel testing is nearly 3 times faster. I > > think after these changes land we can make parallel execution the > > default. > > Ping. Thanks, applied to perf-tools-next, - Arnaldo
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index 92ccca9574ca..32df34dda9cd 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -2881,10 +2881,10 @@ static int __cmd_record(struct record *rec, int argc, const char **argv) } #endif zstd_fini(&session->zstd_data); - perf_session__delete(session); - if (!opts->no_bpf_event) evlist__stop_sb_thread(rec->sb_evlist); + + perf_session__delete(session); return status; }
The session has a header in it which contains a perf env with bpf_progs. The bpf_progs are accessed by the sideband thread and so the sideband thread must be stopped before the session is deleted, to avoid a use after free. This error was detected by AddressSanitizer in the following: ``` ==2054673==ERROR: AddressSanitizer: heap-use-after-free on address 0x61d000161e00 at pc 0x55769289de54 bp 0x7f9df36d4ab0 sp 0x7f9df36d4aa8 READ of size 8 at 0x61d000161e00 thread T1 #0 0x55769289de53 in __perf_env__insert_bpf_prog_info util/env.c:42 #1 0x55769289dbb1 in perf_env__insert_bpf_prog_info util/env.c:29 #2 0x557692bbae29 in perf_env__add_bpf_info util/bpf-event.c:483 #3 0x557692bbb01a in bpf_event__sb_cb util/bpf-event.c:512 #4 0x5576928b75f4 in perf_evlist__poll_thread util/sideband_evlist.c:68 #5 0x7f9df96a63eb in start_thread nptl/pthread_create.c:444 #6 0x7f9df9726a4b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 0x61d000161e00 is located 384 bytes inside of 2136-byte region [0x61d000161c80,0x61d0001624d8) freed by thread T0 here: #0 0x7f9dfa6d7288 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52 #1 0x557692978d50 in perf_session__delete util/session.c:319 #2 0x557692673959 in __cmd_record tools/perf/builtin-record.c:2884 #3 0x55769267a9f0 in cmd_record tools/perf/builtin-record.c:4259 #4 0x55769286710c in run_builtin tools/perf/perf.c:349 #5 0x557692867678 in handle_internal_command tools/perf/perf.c:402 #6 0x557692867a40 in run_argv tools/perf/perf.c:446 #7 0x557692867fae in main tools/perf/perf.c:562 #8 0x7f9df96456c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 ``` Fixes: 657ee5531903 ("perf evlist: Introduce side band thread") Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/builtin-record.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)