Re: [PATCH v2] perf test: perftool-testsuite-report fails on s390

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Uninitialized `perf_config_tmp` leads to arbitrary file deletion via environment injection on non-s390x architectures.
--

--- Patch [1]: [PATCH v2] perf test: perftool-testsuite-report fails on s390 ---
commit f7fabccf09a26131ccc43cc43ec7cc1971402aa3
Author: Thomas Richter <tmricht@linux.ibm.com>

perf test: perftool-testsuite-report fails on s390

This commit addresses a timeout failure in the perftool-testsuite-report
test on s390 architectures. It creates a temporary perf configuration file
to suppress addr2line warnings specifically for s390x systems.

> diff --git a/tools/perf/tests/shell/perftool-testsuite_report.sh b/tools/perf/tests/shell/perftool-testsuite_report.sh
> --- a/tools/perf/tests/shell/perftool-testsuite_report.sh
> +++ b/tools/perf/tests/shell/perftool-testsuite_report.sh
> @@ -6,6 +6,14 @@ test -d "$(dirname "$0")/base_report" || exit 2
>  cd "$(dirname "$0")/base_report" || exit 2
>  status=0
>  
> +# On s390 the default timeout for addr2line is too short, disable warnings
> +if [ $(uname -m) = s390x ]
> +then
> +	perf_config_tmp=$(mktemp /tmp/.perfconfig_XXXXX)
> +	export PERF_CONFIG="${perf_config_tmp}"
> +	perf config 'core.addr2line-disable-warn = true'
> +fi
> +
>  PERFSUITE_RUN_DIR=$(mktemp -d /tmp/"$(basename "$0" .sh)".XXX)

[ ... ]

> @@ -19,5 +27,6 @@ if ! [ "$PERFTEST_KEEP_LOGS" = "y" ]; then
>  	rm -rf "$PERFSUITE_RUN_DIR"
>  fi
>  
> +[ -n "${perf_config_tmp}" ] && rm -f "${perf_config_tmp}"

[Severity: High]
Could this lead to unintended file deletion on non-s390x architectures?

Since perf_config_tmp is only conditionally initialized if the architecture
is s390x, any value passed in via the environment will be retained on other
systems.

If a user or orchestration environment sets perf_config_tmp before running
the script, this cleanup path will execute rm -f against that arbitrary
file path.

Would it be safer to explicitly initialize perf_config_tmp to an empty
string at the start of the script?

>  test $status -ne 0 && exit 1
>  exit 0

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709073024.3778237-1-tmricht@linux.ibm.com?part=1