Re: [PATCH v6 09/59] perf data: Add open flag

sashiko-bot
2026-04-25T18:20:04Z

Sashiko AI review found 1 potential issue(s):
- [High] Uninitialized Memory Bug in `perf_data` array causing undefined behavior.
--

commit 3ce27148af7a0598a2cfebfa260efe575e34351e
Author: Ian Rogers <irogers@google.com>

perf data: Add open flag

This commit adds an open flag to struct perf_data to track whether the
underlying file or directory has been opened. This avoids double opens
and ensures only open files are closed.

> diff --git a/tools/perf/util/data.h b/tools/perf/util/data.h
> index 8299fb5fa7da..76f57f60361f 100644
> --- a/tools/perf/util/data.h
> +++ b/tools/perf/util/data.h

[ ... ]

> @@ -50,6 +50,8 @@ struct perf_data {
> const char *path;
> /** @file: Underlying file to be used. */
> struct perf_data_file file;
> + /** @open: Has the file or directory been opened. */
> + bool open;
> /** @is_pipe: Underlying file is a pipe. */
> bool is_pipe;
> /** @is_dir: Underlying file is a directory. */

[ ... ]

> diff --git a/tools/perf/util/data.c b/tools/perf/util/data.c
> index 94dc534a7386..17baf71897d1 100644
> --- a/tools/perf/util/data.c
> +++ b/tools/perf/util/data.c

[ ... ]

> @@ -360,9 +362,16 @@ static int open_dir(struct perf_data *data)
>
> int perf_data__open(struct perf_data *data)
> {
> - if (check_pipe(data))
> + int ret;
> +
> + if (data->open)
> return 0;

Could the new open flag be uninitialized in some call paths?

Looking at perf_sched__schedstat_diff() in tools/perf/builtin-sched.c, the
data array is allocated on the stack:

tools/perf/builtin-sched.c:perf_sched__schedstat_diff() {
...
struct perf_data data[2];
...
data[0].path = sched->old_file;
data[0].mode = PERF_DATA_MODE_READ;
...
}

Since the struct isn't zero-initialized, could data[0].open and data[1].open
contain stack garbage?

If data->open happens to evaluate to true, perf_data__open() will return 0
without actually opening the file or initializing the file descriptor.

This could cause __perf_session__new() to read from an uninitialized file
descriptor.

I see this regression is fixed later in the series by commit
81cf4f4c4c1266ce2e1faa604bb84ef9cfc8657f (perf sched stats: Fix segmentation
faults in diff mode), but I wanted to point it out here.

> +
> + if (check_pipe(data)) {
> + data->open = true;
> + return 0;
> + }
> +
> /* currently it allows stdio for pipe only */

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425174858.3922152-1-irogers@google.com?part=9