Re: [PATCH v2 3/3] DO NOT MERGE: selftests: perf_events: Add device memory callchain unwinding test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The test silently passes if `/dev/mem` mmap fails or `perf record` encounters an error.
- [Medium] Race condition due to missing synchronization between child process initialization and `perf record`.
- [Medium] The temporary directory and `perf.data` file are permanently leaked on every test run.
- [Medium] The child process can become a runaway process consuming 100% CPU if the parent is terminated.
--

commit c99f3e3dd4b06157e6d6eea6478e0ee3ffe3951a
Author: Fredrik Markstrom <fredrik.markstrom@est.tech>
DO NOT MERGE: selftests: perf_events: Add device memory callchain unwinding test

This commit adds a kselftest that uses perf record to sample a process
that has its frame pointer pointing into a device memory mapping created
via /dev/mem. The test is designed to verify that the kernel survives
without crashing due to synchronous external aborts.

> diff --git a/tools/testing/selftests/perf_events/test_perf_vmio.c b/tools/testing/selftests/perf_events/test_perf_vmio.c
> new file mode 100644
> index 0000000000000..c9ceee3f15927
> --- /dev/null
> +++ b/tools/testing/selftests/perf_events/test_perf_vmio.c
[ ... ]
> +	/* Fork a spinner child with FP pointing into device memory */
> +	spin_pid = fork();
> +	if (spin_pid < 0) {
> +		close(fd);
> +		rmdir(tmpdir);
> +		ASSERT_GE(spin_pid, 0);
> +	}
> +	if (spin_pid == 0) {

If the parent test process crashes or is killed by a test runner timeout
before executing kill(spin_pid, SIGKILL), will the child process be orphaned
and run indefinitely in the busy-wait loop?

Could prctl(PR_SET_PDEATHSIG, SIGKILL) be used here to ensure the child is
automatically killed if the parent dies?

> +		/*
> +		 * mmap /dev/mem in the child so remap_pfn_range populates
> +		 * PTEs directly. fork() does not copy PTEs for VM_PFNMAP
[ ... ]
> +		 */
> +		m = mmap(NULL, off + page_size, PROT_READ | PROT_WRITE,
> +			 MAP_SHARED, fd, page);
> +		if (m == MAP_FAILED)
> +			_exit(2);

If this mmap() fails (for example, due to CONFIG_STRICT_DEVMEM), the child
exits immediately. Since the parent ignores the child's exit status, does
this cause the test to silently pass without ever exercising the
vulnerability?

> +		fp = (char *)m + off;
> +		__asm__ volatile(
> +			"mov x29, %0\n"
> +			"1: b 1b\n"
> +			: : "r"(fp) : "x29", "memory");
> +		_exit(0);
> +	}
> +
> +	/* Launch perf to sample the spinner */
> +	snprintf(pid_str, sizeof(pid_str), "%d", spin_pid);
> +
> +	perf_pid = fork();

Is there a race condition here between the child process initialization and
perf record?

If the system is heavily loaded, perf record might finish its 3-second
sleep before the child process completes the mmap() and reaches the
assembly loop.

Could we add synchronization, such as a pipe or file lock, to ensure the
child is ready before perf starts sampling?

[ ... ]
> +	waitpid(perf_pid, &pst, 0);
> +
> +	kill(spin_pid, SIGKILL);
> +	waitpid(spin_pid, NULL, 0);
> +	close(fd);
> +
> +	/* Clean up perf output */
> +	rmdir(tmpdir);

Does rmdir() fail with ENOTEMPTY because perf record generated a perf.data
file inside the directory?

Since unlink() is not called on the perf.data file, do the directory and
file leak on the file system after every test run?

> +
> +	if (!WIFEXITED(pst))
> +		SKIP(return, "perf terminated abnormally");
> +	if (WEXITSTATUS(pst) == 1)
> +		SKIP(return, "perf not available");
> +
> +	/*
> +	 * The real test is that the kernel survived. If we got here
> +	 * without a synchronous external abort, the guard worked.
> +	 */
> +	TH_LOG("kernel survived perf sampling with FP in device memory");

If the child exited early because mmap() failed, perf record will fail to
attach to the zombie process and might exit with a code other than 1 (such
as 255).

Because the check here only looks for WEXITSTATUS(pst) == 1, does this
allow other perf failures to bypass the check and incorrectly log that the
test passed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260430-master-with-pfix-v3-v2-0-bd526ec04a75@est.tech?part=3