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

sashiko-bot
2026-04-25T23:14:31Z

Sashiko AI review found 2 potential issue(s):
- [High] Using `--switch-output` alongside stdout pipe output triggers a Use-After-Close and File Descriptor reuse bug that can silently corrupt profiling data.
- [High] In `perf_data__switch()`, a failed `rename()` fallback destroys the existing data file via an `O_TRUNC` overwrite, causing severe data loss.
--

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

perf data: Add open flag

This commit introduces an open flag to the perf_data structure to track
the open state of files and directories. It updates the open and close
routines to check and maintain this state, aiming to prevent double opens
and ensure only opened files are closed.

> 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
[ ... ]
> @@ -457,6 +474,7 @@ int perf_data__switch(struct perf_data *data,
>
> if (!at_exit) {
> perf_data_file__close(&data->file);
> + data->open = false;
> ret = perf_data__open(data);

Does this sequence accidentally close standard output and corrupt subsequent
file descriptor allocations if the output is directed to a pipe?

If output is a stdout pipe (for example, using perf record -o -), the call
to perf_data_file__close() will close STDOUT_FILENO (FD 1). When this
reopens the data file, check_pipe() will assign data->file.fd back to
STDOUT_FILENO without performing a system open or dup. Since FD 1 was just
closed, the next unrelated file or socket opened by the process will likely
receive FD 1. Subsequent writes to data->file.fd might then silently corrupt
that unrelated file with the profiling data stream.

Additionally, is it possible for this fallback path to unintentionally
truncate the existing data file and cause data loss?

Earlier in this function, if rename() fails, a warning is printed and a
comment says to continue filling the original file. However, because the
rename failed, data->path still points to the original file:

if (rename(data->path, *new_filepath))
pr_warning("Failed to rename...");

if (!at_exit) {
perf_data_file__close(&data->file);
data->open = false;
ret = perf_data__open(data);

When perf_data__open() is called here, it eventually calls
open_file_write(), which strictly passes the O_TRUNC flag. Will this
truncate the original file to 0 bytes, destroying all previously
collected profiling data before the subsequent seek?

> if (ret < 0)
> goto out;

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