Re: [PATCH 28/28] perf test: Add truncated perf.data robustness test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete crash signal detection excludes SIGILL and SIGTRAP.
- [Medium] Trap handler for INT and TERM signals fails to exit the script, causing false test passes on interruption.
--

commit 4510c7f2cdccee6cf7adf69880ade45a0432607e
Author: Arnaldo Carvalho de Melo <acme@redhat.com>

perf test: Add truncated perf.data robustness test

This commit adds a shell test to verify that perf report handles
truncated perf.data files gracefully. It records a workload, truncates
the file at various offsets, and checks the exit code to ensure no
crashes occur.

> diff --git a/tools/perf/tests/shell/data_validation.sh b/tools/perf/tests/shell/data_validation.sh
> new file mode 100755
> index 0000000000000..649f71b6cdb93
> --- /dev/null
> +++ b/tools/perf/tests/shell/data_validation.sh
> @@ -0,0 +1,59 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Test that perf report handles truncated perf.data gracefully
> +# (no crash, no segfault — clean error exit).
> +#
> +# Exercises the bounds checking and minimum-size validation added
> +# by the perf-data-validation hardening series.
> +
> +err=0
> +
> +cleanup() {
> +	rm -f "${perfdata}" "${truncated}"
> +	trap - EXIT TERM INT
> +}
> +trap cleanup EXIT TERM INT

What happens if this script is interrupted by a TERM or INT signal? 

The cleanup function removes the temporary files and unsets the traps,
but it does not explicitly exit. In bash, if a trap handler does not 
terminate the script, execution resumes from the instruction immediately
following the interruption. 

Would the script continue running the truncation loop without the
temporary files, encounter standard file-not-found errors instead of
crashes, and eventually reach the end to exit with err=0, falsely
reporting that the test passed?

[ ... ]

> +	# perf report should exit with an error, not crash.
> +	# Suppress stdout/stderr — we only care about the exit code.
> +	perf report -i "${truncated}" --stdio > /dev/null 2>&1
> +	exit_code=$?
> +
> +	# 139 = SIGSEGV, 134 = SIGABRT, 136 = SIGFPE
> +	if [ ${exit_code} -ge 134 ] && [ ${exit_code} -le 159 ]; then

Does this lower bound of 134 accidentally exclude other severe crash signals?

An uncaught signal in bash results in an exit code of 128 + signal_number.
By starting at 134 for SIGABRT, this excludes SIGILL (signal 4, exit code 132)
and SIGTRAP (signal 5, exit code 133). 

If parsing malformed data causes a jump to a bad pointer resulting in
an illegal instruction, could the script misinterpret the 132 exit code
as a graceful error and falsely pass the test?

> +		echo "FAIL: perf report crashed (signal $((exit_code - 128))) on ${cut_at}-byte truncated file"
> +		err=1
> +	fi
> +done
> +
> +cleanup
> +exit ${err}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260510033424.255812-1-acme@kernel.org?part=28