Re: [PATCH v8 3/4] perf test: Add inject ASLR test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The signal handler `trap_cleanup` incorrectly deletes the temporary directory on interruption under certain conditions, while falsely claiming to preserve it, and prints duplicate messages on SIGINT.
--

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

perf test: Add inject ASLR test

This commit adds a shell script to verify the perf inject --aslr feature.
It covers address remapping, pipe mode, callchain remapping, dropped
samples, and kernel address normalization.

> diff --git a/tools/perf/tests/shell/inject_aslr.sh b/tools/perf/tests/shell/inject_aslr.sh
> new file mode 100755
> index 0000000000000..d921287e849b3
> --- /dev/null
> +++ b/tools/perf/tests/shell/inject_aslr.sh
[ ... ]
> +cleanup() {
> +  local exit_code=$?
> +  if [ "${exit_code}" -ne 0 ] || [ "${err}" -ne 0 ]; then
> +    echo "Test failed! Preserving temp directory: ${temp_dir}"
> +    return
> +  fi
> +  # Check if temp_dir is set and looks sane before removing
> +  if [[ "${temp_dir}" =~ ^/tmp/perf-test-aslr\. ]]; then
> +    rm -rf "${temp_dir}"
> +  fi
> +}
> +
> +trap_cleanup() {
> +  cleanup
> +  exit 1
> +}
> +
> +trap cleanup EXIT
> +trap trap_cleanup TERM INT

[Severity: Medium]
Will this cause the cleanup routine to run twice when a signal is received?

Since trap_cleanup explicitly calls cleanup and then exits with code 1, won't
the exit 1 statement natively trigger the EXIT trap and invoke cleanup a
second time?

If the script receives a targeted signal (like SIGTERM) while the foreground
command successfully completes, the first cleanup execution evaluates the exit
status as successful and irrevocably deletes the temporary directory. The
immediate exit 1 then triggers the second cleanup execution, which evaluates
the failure status and prints the preservation message for a directory that
no longer exists. Under a standard interruption like SIGINT, it prints the
preservation message twice.

Could trap_cleanup simply execute exit 1 and rely entirely on the EXIT trap
to handle unified cleanup?

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