Re: [PATCH 2/3] DO NOT MERGE: arm64: perf: Add skip_vmio parameter to control device memory callchain guard

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The patch embeds a test-only backdoor into core architectural code, exposing a system-wide Denial of Service (DoS) vulnerability.
- [High] The device memory guard is vulnerable to a Time-of-Check to Time-of-Use (TOCTOU) race condition that can lead to an unprivileged Denial of Service.
- [Medium] The `skip_vmio` parameter uses `module_param()` instead of `module_param_unsafe()`, failing to taint the kernel when system integrity is compromised.
--

commit 7350d20819d5cd55953c6253c2ca394a596ece60
Author: Fredrik Markstrom <fredrik.markstrom@est.tech>

DO NOT MERGE: arm64: perf: Add skip_vmio parameter to control device memory callchain guard

This commit introduces a skip_vmio module parameter to bypass the device
memory guard during user callchain unwinding. It allows administrators to
disable the protection dynamically for testing external abort paths.

> diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
> index 6426a307b8f8..ebe909012e4e 100644
> --- a/arch/arm64/kernel/stacktrace.c
> +++ b/arch/arm64/kernel/stacktrace.c

[ ... ]

> @@ -112,6 +113,10 @@ static bool addr_is_device_mem(unsigned long addr)
>  #endif
>  }
>  
> +static bool skip_vmio = true;
> +module_param(skip_vmio, bool, 0644);
> +MODULE_PARM_DESC(skip_vmio, "Skip device memory during user callchain unwinding");

Does introducing this parameter expose the system to an unprivileged denial
of service? 

If an administrator disables this guard for testing, it globally removes the
device memory protection. An unprivileged user profiling their process could
then point their frame pointer into a mapped device memory region, triggering
a synchronous external abort and kernel panic. 

Test mechanisms that deliberately enable hardware panics are typically
restricted to isolated testing modules or conditionally compiled out of
production kernels.

Also, since disabling this guard deliberately introduces a known crash
vector, should this use module_param_unsafe() instead of module_param()?

Using the unsafe variant would taint the kernel, ensuring that subsequent
crash dumps clearly indicate the kernel was running in a compromised state
rather than experiencing a genuine hardware failure.

>  enum kunwind_source {
>  	KUNWIND_SOURCE_UNKNOWN,

[ ... ]

> @@ -616,7 +621,7 @@ unwind_user_frame(struct frame_tail __user *tail, void *cookie,
>  	if (!access_ok(tail, sizeof(buftail)))
>  		return NULL;
>  
> -	if (addr_is_device_mem((unsigned long)tail))
> +	if (READ_ONCE(skip_vmio) && addr_is_device_mem((unsigned long)tail))
>  		return NULL;
>  
>  	pagefault_disable();

This is an existing issue with the guard being modified here, but is there a
time-of-check to time-of-use race condition between the lockless check and
the memory read?

Because addr_is_device_mem() performs a lockless page table walk, a
concurrent thread could potentially remap the target virtual address to
device memory in the window before the memory is accessed:

Thread 1                               Thread 2
addr_is_device_mem(tail)
// returns false
                                       mmap(tail, ..., MAP_SHARED, fd, 0)
                                       // remaps to device memory
pagefault_disable()
__copy_from_user_inatomic(...)
// triggers synchronous external abort

Could this allow an attacker to bypass the guard and force an illegal device
memory read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428-master-with-pfix-v3-v1-0-c384d3e53092@est.tech?part=2