Re: [PATCH v8 06/11] arm64/sframe: Validate IP addresses

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] unwind: noinstr violation via static inline callees in sframe_find()
--

commit e12d5712c137b31d5aeba1eafd95262908a336d3
Author: Dylan Hatch <dylanbhatch@xxxxxxxxxx>

arm64/sframe: Validate IP addresses

This commit adds validation for instruction pointers during sframe unwinding.
It introduces architecture-specific checks for arm64 to ensure the IPs fall
within valid kernel or module text ranges, handling special sections like
.exit.text and .rodata.text.

[ ... ]
> diff --git a/arch/arm64/include/asm/unwind_sframe.h b/arch/arm64/include/asm/unwind_sframe.h
> new file mode 100644
> index 0000000000000..1b45d328c746f
> --- /dev/null
> +++ b/arch/arm64/include/asm/unwind_sframe.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _ASM_ARM64_UNWIND_SFRAME_H
> +#define _ASM_ARM64_UNWIND_SFRAME_H
> +
> +#include <linux/module.h>
> +#include <linux/sframe.h>
> +#include <asm/sections.h>
> +
> +static __always_inline bool sframe_is_kernel_ip_valid(unsigned long ip)
> +{
> +	if (__is_kernel_text(ip) || is_kernel_inittext(ip))
> +		return true;

[Severity: Low]
Since sframe_find() is marked noinstr and calls this helper, marking
sframe_is_kernel_ip_valid() as __always_inline helps prevent it from being
outlined. However, __is_kernel_text() and is_kernel_inittext() are standard
static inline functions. Could the compiler decide to outline these nested
helpers into the instrumentable .text section?

If that happens, objtool validation might fail during the build, and it could
theoretically result in recursive instrumentation faults (e.g., KASAN) if
called during NMI stack unwinding.

> +
> +	/* .exit.text is retained in vmlinux on arm64. */
> +	if (ip >= (unsigned long)__exittext_begin &&
[ ... ]
> diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
> index 33883408581da..c95fcb63e7eaf 100644
> --- a/kernel/unwind/sframe.c
> +++ b/kernel/unwind/sframe.c
[ ... ]
> @@ -44,6 +44,45 @@ struct sframe_fre_internal {
[ ... ]
> +#ifdef CONFIG_MODULES
> +
> +static __always_inline bool sframe_is_sec_module_ip_valid(struct sframe_section *sec,
> +							  unsigned long ip)
> +{
> +	struct module *mod = container_of(sec, struct module, sframe_sec);
> +
> +	return within_module_mem_type(ip, mod, MOD_TEXT) ||
> +		within_module_mem_type(ip, mod, MOD_INIT_TEXT);

[Severity: Low]
Similarly, within_module_mem_type() is a regular static inline function
called from the noinstr path. Does this present the same outlining risk
as the kernel text helpers above?

> +}
> +
> +#else

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007114335.440322-1-dylanbhatch@xxxxxxxxxx?part=6