On Mon, Apr 20, 2026 at 5:31 AM Jens Remus <[email protected]> wrote:
>
> On 4/20/2026 7:02 AM, Dylan Hatch wrote:
> > On Thu, Apr 16, 2026 at 8:04 AM Jens Remus <[email protected]> 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;
I agree this is correct. Is this a fix that would be folded into your
previous patch series?
>
> > .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):
Thanks for the suggestion! I'll look into incorporating this.
>
> 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
It looks to me like .rodata.text only exists for vmlinux. I wonder if
in sframe_func_start_addr_valid we can just use the global
_srodatatext and _erodatatext after identifying if an sframe_section
corresponds to vmlinux (kernel_sfsec)? That way we don't need to add
these extra fields.
>
> 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)
> [email protected] / [email protected]
>
> 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/
>
Thanks,
Dylan
