Re: [RFC PATCH v4 5/5] perf test: Add test validating trace.dat generated by 'perf data convert --to-trace-dat'

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Insecure temporary file reuse creates a time-of-check to time-of-use (TOCTOU) race condition and symlink attack vulnerability.
- [Medium] The commit message claims to test the 'cycles' event, but the script uses 'cpu-clock'.
- [Medium] The script leaks a temporary file (`${perfdata}.old`) in `/tmp` because `perf record` automatically backs up the existing non-empty output file.
--

--- Patch [5]: [RFC PATCH v4 5/5] perf test: Add test validating trace.dat generated by 'perf data convert --to-trace-dat' ---
commit 9c32c2021bfda9ef574a5486cbcea176af233faa
Author: Tanushree Shah <[email protected]>

perf test: Add test validating trace.dat generated by 'perf data convert --to-trace-dat'

Add a shell test covering perf data convert --to-trace-dat,
alongside the existing --to-json and --to-ctf tests.

The test:
- skips if perf isn't linked with libtraceevent
- converts a plain sched:sched_switch recording to trace.dat
- converts a pipe-mode recording (perf record -o - | perf data
  convert -i -)
> - converts a recording with both tracepoint and non-tracepoint
>   events (sched:sched_switch,cycles)

[Severity: Medium]
Is this description accurate? The commit message mentions the "cycles"
event, but the code later tests the "cpu-clock" event instead.

- validates the resulting file with trace-cmd report if trace-cmd
  is installed, otherwise falls back to a basic non-empty-file check

Signed-off-by: Tanushree Shah <[email protected]>

> diff --git a/tools/perf/tests/shell/test_perf_data_converter_tracepoints_trace_dat.sh b/tools/perf/tests/shell/test_perf_data_converter_tracepoints_trace_dat.sh
> new file mode 100755
> index 0000000000000..1c0bce270c389
> --- /dev/null
> +++ b/tools/perf/tests/shell/test_perf_data_converter_tracepoints_trace_dat.sh
> @@ -0,0 +1,173 @@
> +#!/bin/bash
> +# 'perf data convert --to-trace-dat' command test
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Copyright 2026, IBM Corporation
> +# Author: Tanushree Shah <[email protected]>
> +
> +set -e
> +
> +err=0
> +
> +perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> +
> +cleanup()
> +{
> +	rm -f "${perfdata}"

[Severity: Medium]
Does this code leak the backup file created by perf?

When perf record is called a second time in test_trace_converter_mixed_events()
with -o "$perfdata", it detects that the file already exists and has content.
perf record automatically renames the existing file to "${perfdata}.old" before
creating a new one.

Since cleanup() only removes "${perfdata}", does the .old file get left
behind in /tmp?

> +	rm -f "${result}"
> +	trap - exit term int
> +}
> +
[ ... ]
> +test_trace_converter_pipe()
> +{
> +	echo "Testing Perf Data Conversion Command to trace.dat (Pipe mode)"
> +
> +	rm -f "$result"
> +
> +	if ! perf record -e sched:sched_switch -a -o - -- sleep 0.1 | \
> +	   perf data convert --to-trace-dat "$result" --force -i -

[Severity: Medium]
Does this code create a time-of-check to time-of-use race condition?

The script securely creates a temporary file using mktemp, but explicitly
deletes it here with rm -f "$result". 

Since the predictable and unsecured path is then immediately reused for
perf data convert, could a local attacker insert a symlink during this window,
causing perf data convert to overwrite arbitrary files?

> +	then
> +		echo "Perf Data Converter Command to trace.dat (Pipe mode) [FAILED]"
> +		err=1
> +		return
> +	fi
> +
[ ... ]
> +test_trace_converter_mixed_events()
> +{
> +	echo "Testing Perf Data Conversion with tracepoint and non-tracepoint events"
> +
> +	rm -f "$result"
> +
> +	# Record both tracepoint and non-tracepoint events
> +	if ! perf record -e sched:sched_switch,cpu-clock -a -o "$perfdata" -- sleep 0.1
> +	then
> +		echo "Failed to record perf data (mixed events)"
> +		err=1
> +		return
> +	fi
> +
> +	if ! perf data convert --to-trace-dat "$result" --force -i "$perfdata"

[Severity: Medium]
Does this code repeat the symlink race condition mentioned above?

The securely created $result file is again explicitly deleted and its
path is reused here.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5