2024-03-01 07:53:25

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] perf record: Delete session after stopping sideband thread

On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <[email protected]> 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 <[email protected]>

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


2024-03-20 04:42:55

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] perf record: Delete session after stopping sideband thread

On Thu, Feb 29, 2024 at 11:50 PM Ian Rogers <[email protected]> wrote:
>
> On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <[email protected]> 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 <[email protected]>
>
> 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

2024-03-20 14:50:09

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v1 1/4] perf record: Delete session after stopping sideband thread

On Tue, Mar 19, 2024 at 09:42:26PM -0700, Ian Rogers wrote:
> On Thu, Feb 29, 2024 at 11:50 PM Ian Rogers <[email protected]> wrote:
> >
> > On Thu, Feb 29, 2024 at 11:47 PM Ian Rogers <[email protected]> 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 <[email protected]>
> >
> > 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