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 <[email protected]> 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/[email protected]?part=6
