Re: [PATCH v5 12/19] perf test cs-etm: Skip if not root

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The script is vulnerable to a TOCTOU symlink attack that allows a local attacker to overwrite arbitrary system files with root privileges.
--

commit 31448575355dfbf022d3a5103f47303bea93bbf4
Author: James Clark <james.clark@linaro.org>

perf test cs-etm: Skip if not root

Use the common idiom for skipping tests if not running as root, which is
required for these tests.

> diff --git a/tools/perf/tests/shell/test_arm_coresight.sh b/tools/perf/tests/shell/test_arm_coresight.sh
> --- a/tools/perf/tests/shell/test_arm_coresight.sh
> +++ b/tools/perf/tests/shell/test_arm_coresight.sh
> @@ -20,6 +20,12 @@ skip_if_no_cs_etm_event() {
>  
>  skip_if_no_cs_etm_event || exit 2
>  
> +if [ "$(id -u)" != 0 ]; then
> +	# Requires root for -C and system wide tests
> +	echo "[Skip] No root permission"
> +	exit 2
> +fi
> +
>  perfdata=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
>  file=$(mktemp /tmp/temporary_file.XXXXX)

[Severity: High]
This is a pre-existing issue, but does enforcing root execution expose these
temporary files to a symlink attack?

While the temporary files are securely generated with mktemp in the
world-writable /tmp/ directory, the script breaks these guarantees before
writing to them.

Looking at record_touch_file() in tools/perf/tests/shell/test_arm_coresight.sh:

record_touch_file() {
	echo "Recording trace (only user mode) with path: CPU$2 => $1"
	rm -f $file
	perf record -o ${perfdata} -e cs_etm/@$1/u --per-thread \
		-- taskset -c $2 touch $file > /dev/null 2>&1
}

This explicitly deletes the safe file and executes touch as root.

Additionally, looking at arm_cs_etm_system_wide_test():

arm_cs_etm_system_wide_test() {
	echo "Recording trace with system wide mode"
	perf record -o ${perfdata} -e cs_etm// -a -- ls > /dev/null 2>&1

Successive perf record calls rename the existing ${perfdata} to .old, freeing
the securely generated filename.

In both cases, could a local attacker use inotify to detect the deletion or
rename, and insert a symlink pointing to an arbitrary file (like /etc/shadow)?

Since this patch explicitly enforces the script to run as root, won't
subsequent file operations by perf record (which internally uses open with
O_CREAT | O_RDWR | O_TRUNC without O_NOFOLLOW) follow the symlink and
overwrite system files with root privileges?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-james-cs-context-tracking-fix-v5-0-d53a7d096a19@linaro.org?part=12