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

On Mon, Apr 20, 2026 at 5:31=E2=80=AFAM Jens Remus <[email protected]> w=
rote:
>
> On 4/20/2026 7:02 AM, Dylan Hatch wrote:
> > On Thu, Apr 16, 2026 at 8:04=E2=80=AFAM Jens Remus <[email protected]=
m> 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.tex=
t.
> >>> 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 >=3D 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
> >                 . =3D 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/s=
ections.h
> @@ -23,6 +23,7 @@ extern char __irqentry_text_start[], __irqentry_text_en=
d[];
>  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 *s=
ec,
> +                                               unsigned long func_addr)
> +{
> +       return (sec->text_start >=3D func_addr && func_addr < sec->text_e=
nd) ||
> +              (sec->rodatatext_start >=3D 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  =3D (unsigned long)_srodatatext;
> +       kernel_sfsec->rodatatext_end    =3D (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 =3D .;
>                 TRAMP_TEXT
>                 HIBERNATE_TEXT
>                 KEXEC_TEXT
>                 IDMAP_TEXT
>                 . =3D ALIGN(PAGE_SIZE);
>         }
> +       _erodatatext =3D .;
>
>         idmap_pg_dir =3D .;
>         . +=3D 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 *s=
ec,
> +                                                unsigned long func_addr)
> +{
> +       return (sec->text_start <=3D func_addr && func_addr < sec->text_e=
nd);
> +}
> +#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_s=
ection *sec,
>                   sizeof(struct sframe_fde_v3), Efault);
>
>         func_addr =3D 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 =3D sec->fres_start + _fde.fres_off;
> @@ -696,13 +708,6 @@ static int sframe_validate_section(struct sframe_sec=
tion *sec)
>                 int ret;
>
>                 ret =3D 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 =3D=3D 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 =3D true;
>  }
>
> Regards,
> Jens
> --
> Jens Remus
> Linux on Z Development (D3303)
> [email protected] / [email protected]
>
> IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsra=
ts: Wolfgang Wendt; Gesch=C3=A4ftsf=C3=BChrung: David Faller; Sitz der Gese=
llschaft: Ehningen; Registergericht: Amtsgericht Stuttgart, HRB 243294
> IBM Data Privacy Statement: https://www.ibm.com/privacy/
>

Thanks,
Dylan