Re: [PATCH] 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:
- [Medium] Uninitialized variable `perf_config_tmp` is passed to `rm -f`, leading to potential arbitrary file deletion via environment variable injection.
--

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

perf test: perftool-testsuite-report fails on s390

This commit fixes a test failure on s390 by creating a temporary perf
configuration file that disables addr2line warnings, working around a
timeout issue.

> diff --git a/tools/perf/tests/shell/perftool-testsuite_report.sh b/tools/perf/tests/shell/perftool-testsuite_report.sh
> index a8cf75b4e77ec..997573f0e74c5 100755
> --- 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
> +

[ ... ]

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

[Severity: Medium]
Is it possible for perf_config_tmp to be read from the environment on
non-s390x architectures?

Since perf_config_tmp is only initialized inside the s390x if-block above,
if this script runs on another architecture, the variable remains
uninitialized locally. Could an inherited environment variable of the same
name cause an unintended file to be deleted here?

Additionally, executing rm -f "" on non-s390x systems is sloppy and can
sometimes produce spurious errors depending on the rm implementation. Would
it be better to unconditionally initialize the variable to empty at the start
of the script, or check if it is set before calling rm?

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

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