linux-kernel - Re: [PATCH] perf record: skip synthesize event when open evsel failed

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fWupb62_QKM3bZO9K9yeJqC2H-bdi6dQNM7zAsLTJoDow@mail.gmail.com>
Date: Thu, 23 Oct 2025 09:08:52 -0700
From: Ian Rogers <irogers@...gle.com>
To: Shuai Xue <xueshuai@...ux.alibaba.com>
Cc: alexander.shishkin@...ux.intel.com, peterz@...radead.org, 
	james.clark@....com, leo.yan@...aro.org, mingo@...hat.com, 
	baolin.wang@...ux.alibaba.com, acme@...nel.org, mark.rutland@....com, 
	jolsa@...nel.org, namhyung@...nel.org, adrian.hunter@...el.com, 
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org, 
	nathan@...nel.org, bpf@...r.kernel.org
Subject: Re: [PATCH] perf record: skip synthesize event when open evsel failed

On Wed, Oct 22, 2025 at 6:50 PM Shuai Xue <xueshuai@...ux.alibaba.com> wrote:
>
> When using perf record with the `--overwrite` option, a segmentation fault
> occurs if an event fails to open. For example:
>
>   perf record -e cycles-ct -F 1000 -a --overwrite
>   Error:
>   cycles-ct:H: PMU Hardware doesn't support sampling/overflow-interrupts. Try 'perf stat'
>   perf: Segmentation fault
>       #0 0x6466b6 in dump_stack debug.c:366
>       #1 0x646729 in sighandler_dump_stack debug.c:378
>       #2 0x453fd1 in sigsegv_handler builtin-record.c:722
>       #3 0x7f8454e65090 in __restore_rt libc-2.32.so[54090]
>       #4 0x6c5671 in __perf_event__synthesize_id_index synthetic-events.c:1862
>       #5 0x6c5ac0 in perf_event__synthesize_id_index synthetic-events.c:1943
>       #6 0x458090 in record__synthesize builtin-record.c:2075
>       #7 0x45a85a in __cmd_record builtin-record.c:2888
>       #8 0x45deb6 in cmd_record builtin-record.c:4374
>       #9 0x4e5e33 in run_builtin perf.c:349
>       #10 0x4e60bf in handle_internal_command perf.c:401
>       #11 0x4e6215 in run_argv perf.c:448
>       #12 0x4e653a in main perf.c:555
>       #13 0x7f8454e4fa72 in __libc_start_main libc-2.32.so[3ea72]
>       #14 0x43a3ee in _start ??:0
>
> The --overwrite option implies --tail-synthesize, which collects non-sample
> events reflecting the system status when recording finishes. However, when
> evsel opening fails (e.g., unsupported event 'cycles-ct'), session->evlist
> is not initialized and remains NULL. The code unconditionally calls
> record__synthesize() in the error path, which iterates through the NULL
> evlist pointer and causes a segfault.
>
> To fix it, move the record__synthesize() call inside the error check block, so
> it's only called when there was no error during recording, ensuring that evlist
> is properly initialized.
>
> Fixes: 4ea648aec019 ("perf record: Add --tail-synthesize option")
> Signed-off-by: Shuai Xue <xueshuai@...ux.alibaba.com>

This looks great! I wonder if we can add a test, perhaps here:
https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/record.sh?h=perf-tools-next#n435
something like:
```
$ perf record -e foobar -F 1000 -a --overwrite -o /dev/null -- sleep 0.1
```
in a new test subsection for test_overwrite? foobar would be an event
that we could assume isn't present. Could you help with a test
covering the problems you've uncovered and perhaps related flags?

Thanks,
Ian

> ---
>  tools/perf/builtin-record.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index d76f01956e33..b1fb87016d5a 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -2883,11 +2883,11 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
>                 rec->bytes_written += off_cpu_write(rec->session);
>
>         record__read_lost_samples(rec);
> -       record__synthesize(rec, true);
>         /* this will be recalculated during process_buildids() */
>         rec->samples = 0;
>
>         if (!err) {
> +               record__synthesize(rec, true);
>                 if (!rec->timestamp_filename) {
>                         record__finish_output(rec);
>                 } else {
> --
> 2.39.3
>