Re: [PATCH v3 7/8] sframe: Introduce in-kernel SFRAME_VALIDATION.

[Jens Remus <jremus@linux.ibm.com>]

On 4/20/2026 7:02 AM, Dylan Hatch wrote:
> On Thu, Apr 16, 2026 at 8:04 AM Jens Remus <jremus@linux.ibm.com> wrote:
>> On 4/6/2026 8:49 PM, Dylan Hatch wrote:

>>> Generalize the __safe* helpers to support a non-user-access code path.
>>> Allow for kernel FDE read failures due to the presence of .rodata.text.
>>> This section contains code that can't be executed by the kernel
>>> direclty, and thus lies ouside the normal kernel-text bounds.
>>
>> Nits: s/direclty/directly/ s/ouside/outside/
>>
>> Could you please explain the issue?  How/why does .sframe for
>> .rodata.text pose an issue for .sframe verification?
> 
> __read_fde checks that the fde_addr it extracts is within the bounds
> of sec->text_start and sec->text_end. In the case of the vmlinux

Looking at the existing check in __read_fde(), do you agree that it is
wrong, as sec->text_end IIUC points behind .text and thus the check
should be:

	if (func_addr < sec->text_start || func_addr >= sec->text_end)
        	return -EINVAL;

> .sframe section, this is _stext and _etext. However on arm64, there is
> an .rodata.text section that lies outside this range. From
> arch/arm64/kernel/vmlinux.lds.S:
> 
>         /* code sections that are never executed via the kernel mapping */
>         .rodata.text : {
>                 TRAMP_TEXT
>                 HIBERNATE_TEXT
>                 KEXEC_TEXT
>                 IDMAP_TEXT
>                 . = ALIGN(PAGE_SIZE);
>         }
> 
> So __read_fde fails for functions in this section. Under normal SFrame
> usage for unwinding, we should never need to look up a PC value in
> these functions because they will never be executed by the kernel.
> However, we still hit this error when validating all FDEs.

Thanks for the explanation!  Could you please improve the commit
message, for instance as follows:

__read_fde() checks that the extracted FDE function start address is
within the bounds of the .text section start and end.  In case of
vmlinux this is _stext and _etext.  However on arm64, .rodata.text
resides outside this range, causing __read_fde() to fail.

> I think ideally we might prevent sframe data from being generated for
> this code (maybe from the linker script somehow?), but I don't know of
> a simple way to do this.

I dont't know of any way to exclude a single function or a whole section
from .sframe generation.  The GNU linker would discard SFrame FDEs and
its FREs for discarded functions.  But in this case the function itself
is not discarded.  As .sframe is not generated separately per section it
is also not possible to discard e.g. .sframe.rodata.

> Alternatively, we can check for FDEs located
> in .rodata.text during validation, but this seems to only be present
> in arm64, so maybe we would need an arch-specific hook to do this? I'm
> open to suggestions.

Maybe that is better than ignoring __read_fde() failures?  I first
thought this would get nasty, but maybe it would not be too bad.
Following is what I came up with (note tabs replaced by spaces due to
copy&paste from terminal):

diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
@@ -23,6 +23,7 @@ extern char __irqentry_text_start[], __irqentry_text_end[];
 extern char __mmuoff_data_start[], __mmuoff_data_end[];
 extern char __entry_tramp_text_start[], __entry_tramp_text_end[];
 extern char __relocate_new_kernel_start[], __relocate_new_kernel_end[];
+extern char _srodatatext[], _erodatatext[];

 static inline size_t entry_tramp_text_size(void)
 {
diff --git a/arch/arm64/include/asm/unwind_sframe.h b/arch/arm64/include/asm/unwind_sframe.h
@@ -2,11 +2,28 @@
 #ifndef _ASM_ARM64_UNWIND_SFRAME_H
 #define _ASM_ARM64_UNWIND_SFRAME_H

+#include <linux/sframe.h>
+
 #ifdef CONFIG_ARM64

 #define SFRAME_REG_SP  31
 #define SFRAME_REG_FP  29

+static inline bool sframe_func_start_addr_valid(struct sframe_section *sec,
+                                               unsigned long func_addr)
+{
+       return (sec->text_start >= func_addr && func_addr < sec->text_end) ||
+              (sec->rodatatext_start >= func_addr && func_addr < sec->rodatatext_end);
+}
+#define sframe_func_start_addr_valid sframe_func_start_addr_valid
+
+static void arch_init_sframe_table(struct sframe_section *kernel_sfsec)
+{
+       kernel_sfsec->rodatatext_start  = (unsigned long)_srodatatext;
+       kernel_sfsec->rodatatext_end    = (unsigned long)_erodatatext;
+}
+#define arch_init_sframe_table arch_init_sframe_table
+
 #endif

 #endif /* _ASM_ARM64_UNWIND_SFRAME_H */
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
@@ -213,12 +213,14 @@ SECTIONS

        /* code sections that are never executed via the kernel mapping */
        .rodata.text : {
+               _srodatatext = .;
                TRAMP_TEXT
                HIBERNATE_TEXT
                KEXEC_TEXT
                IDMAP_TEXT
                . = ALIGN(PAGE_SIZE);
        }
+       _erodatatext = .;

        idmap_pg_dir = .;
        . += PAGE_SIZE;
diff --git a/include/linux/sframe.h b/include/linux/sframe.h
@@ -63,6 +63,10 @@ struct sframe_section {
        unsigned long           sframe_end;
        unsigned long           text_start;
        unsigned long           text_end;
+#if defined(CONFIG_SFRAME_UNWINDER) && defined(CONFIG_ARM64)
+       unsigned long           rodatatext_start;
+       unsigned long           rodatatext_end;
+#endif

        bool                    fdes_sorted;
        unsigned long           fdes_start;
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
@@ -20,11 +20,23 @@
 #include "sframe.h"
 #include "sframe_debug.h"

+#ifndef sframe_func_start_addr_valid
+static inline bool sframe_func_start_addr_valid(struct sframe_section *sec,
+                                                unsigned long func_addr)
+{
+       return (sec->text_start <= func_addr && func_addr < sec->text_end);
+}
+#endif
+
 #ifdef CONFIG_SFRAME_UNWINDER

 static bool sframe_init __ro_after_init;
 static struct sframe_section kernel_sfsec __ro_after_init;

+#ifndef arch_init_sframe_table
+static void arch_init_sframe_table(struct sframe_section *kernel_sfsec) {}
+#endif
+
 #endif /* CONFIG_SFRAME_UNWINDER */

 struct sframe_fde_internal {
@@ -152,7 +164,7 @@ static __always_inline int __read_fde(struct sframe_section *sec,
                  sizeof(struct sframe_fde_v3), Efault);

        func_addr = fde_addr + _fde.func_start_off;
-       if (func_addr < sec->text_start || func_addr > sec->text_end)
+       if (!sframe_func_start_addr_valid(sec, func_addr))
                return -EINVAL;

        fda_addr = sec->fres_start + _fde.fres_off;
@@ -696,13 +708,6 @@ static int sframe_validate_section(struct sframe_section *sec)
                int ret;

                ret = safe_read_fde(sec, i, &fde);
-               /*
-                * Code in .rodata.text is not considered part of normal kernel
-                * text, but there is no easy way to prevent sframe data from
-                * being generated for it.
-                */
-               if (ret && sec->sec_type == SFRAME_KERNEL)
-                       continue;
                if (ret)
                        return ret;

@@ -1031,6 +1036,8 @@ void __init init_sframe_table(void)
        if (WARN_ON(sframe_validate_section(&kernel_sfsec)))
                return;

+       arch_init_sframe_table(&kernel_sfsec);
+
        sframe_init = true;
 }

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
jremus@de.ibm.com / jremus@linux.ibm.com

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/