[RFC PATCH v2 1/3] arm64: unwind: add asynchronous unwind tables to kernel and modules

[Ard Biesheuvel]

On Thu, 5 May 2022 at 22:20, Nick Desaulniers <ndesaulniers at google.com> wrote:
>
> On Thu, May 5, 2022 at 9:10 AM Ard Biesheuvel <ardb at kernel.org> wrote:
> >
> > Enable asynchronous unwind table generation for both the core kernel as
> > well as modules, and emit the resulting .eh_frame sections as init code
> > so we can use the unwind directives for code patching at boot or module
> > load time.
> >
> > This will be used by dynamic shadow call stack support, which will rely
> > on code patching rather than compiler codegen to emit the shadow call
> > stack push and pop instructions.
> >
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
>
> Two minor nits inline below; I don't feel strongly about them either way.
> Reviewed-by: Nick Desaulniers <ndesaulniers at google.com>
>
> > ---
> >  arch/arm64/Kconfig                    |  3 +++
> >  arch/arm64/Makefile                   |  5 +++++
> >  arch/arm64/include/asm/module.lds.h   |  8 ++++++++
> >  arch/arm64/kernel/vmlinux.lds.S       | 16 ++++++++++++++++
> >  drivers/firmware/efi/libstub/Makefile |  1 +
> >  5 files changed, 33 insertions(+)
> >
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 57c4c995965f..b6302f7cd73f 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -354,6 +354,9 @@ config KASAN_SHADOW_OFFSET
> >         default 0xeffffff800000000 if ARM64_VA_BITS_36 && KASAN_SW_TAGS
> >         default 0xffffffffffffffff
> >
> > +config UNWIND_TABLES
> > +       bool
> > +
> >  source "arch/arm64/Kconfig.platforms"
> >
> >  menu "Kernel Features"
> > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> > index 2f1de88651e6..a4c6807ecbaf 100644
> > --- a/arch/arm64/Makefile
> > +++ b/arch/arm64/Makefile
> > @@ -45,8 +45,13 @@ KBUILD_CFLAGS        += $(call cc-option,-mabi=lp64)
> >  KBUILD_AFLAGS  += $(call cc-option,-mabi=lp64)
> >
> >  # Avoid generating .eh_frame* sections.
> > +ifneq ($(CONFIG_UNWIND_TABLES),y)
>
> You know how I feel about negated conditions when there's an else branch...
>
> >  KBUILD_CFLAGS  += -fno-asynchronous-unwind-tables -fno-unwind-tables
> >  KBUILD_AFLAGS  += -fno-asynchronous-unwind-tables -fno-unwind-tables
> > +else
> > +KBUILD_CFLAGS  += -fasynchronous-unwind-tables
> > +KBUILD_AFLAGS  += -fasynchronous-unwind-tables
> > +endif
> >
> >  ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
> >  prepare: stack_protector_prepare
> > diff --git a/arch/arm64/include/asm/module.lds.h b/arch/arm64/include/asm/module.lds.h
> > index 094701ec5500..dbba4b7559aa 100644
> > --- a/arch/arm64/include/asm/module.lds.h
> > +++ b/arch/arm64/include/asm/module.lds.h
> > @@ -17,4 +17,12 @@ SECTIONS {
> >          */
> >         .text.hot : { *(.text.hot) }
> >  #endif
> > +
> > +#ifdef CONFIG_UNWIND_TABLES
> > +       /*
> > +        * Currently, we only use unwind info at module load time, so we can
> > +        * put it into the .init allocation.
> > +        */
> > +       .init.eh_frame : { *(.eh_frame) }
> > +#endif
> >  }
> > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> > index edaf0faf766f..2f4908c8d152 100644
> > --- a/arch/arm64/kernel/vmlinux.lds.S
> > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > @@ -122,6 +122,17 @@ jiffies = jiffies_64;
> >  #define TRAMP_TEXT
> >  #endif
> >
> > +#ifdef CONFIG_UNWIND_TABLES
> > +#define UNWIND_DATA_SECTIONS                           \
> > +       .eh_frame : {                                   \
> > +               __eh_frame_start = .;                   \
> > +               *(.eh_frame)                            \
> > +               __eh_frame_end = .;                     \
> > +       }
> > +#else
> > +#define UNWIND_DATA_SECTIONS
> > +#endif
> > +
> >  /*
> >   * The size of the PE/COFF section that covers the kernel image, which
> >   * runs from _stext to _edata, must be a round multiple of the PE/COFF
> > @@ -150,6 +161,9 @@ SECTIONS
> >         /DISCARD/ : {
> >                 *(.interp .dynamic)
> >                 *(.dynsym .dynstr .hash .gnu.hash)
> > +#ifndef CONFIG_UNWIND_TABLES
> > +               *(.eh_frame)
> > +#endif
>
> If CONFIG_UNWIND_TABLES is unset, then
> `-fno-asynchronous-unwind-tables -fno-unwind-tables` should be set; it
> would be anomalous otherwise.  We could leave this hunk out and we
> should get a warning from `--orphan-handling=warn` linker flag which
> would warn explicitly about such anomalies, rather than produce but
> then silently discard them.
>

Agreed, I'll drop this bit.