Re: [PATCH v5 23/36] arm/build: Explicitly keep .ARM.attributes sections

[Fangrui Song]

On 2020-08-03, 'Nick Desaulniers' via Clang Built Linux wrote:
> On Fri, Jul 31, 2020 at 4:18 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> > In preparation for adding --orphan-handling=warn, explicitly keep the
> > .ARM.attributes section by expanding the existing ELF_DETAILS macro into
> > ARM_DETAILS.
> >
> > Suggested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > Link: https://lore.kernel.org/lkml/CAKwvOdk-racgq5pxsoGS6Vtifbtrk5fmkmnoLxrQMaOvV0nPWw@xxxxxxxxxxxxxx/
> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> > ---
> >  arch/arm/include/asm/vmlinux.lds.h | 4 ++++
> >  arch/arm/kernel/vmlinux-xip.lds.S  | 2 +-
> >  arch/arm/kernel/vmlinux.lds.S      | 2 +-
> >  3 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm/include/asm/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
> > index a08f4301b718..c4af5182ab48 100644
> > --- a/arch/arm/include/asm/vmlinux.lds.h
> > +++ b/arch/arm/include/asm/vmlinux.lds.h
> > @@ -52,6 +52,10 @@
> >                 ARM_MMU_DISCARD(*(__ex_table))                          \
> >                 COMMON_DISCARDS
> >
> > +#define ARM_DETAILS                                                    \
> > +               ELF_DETAILS                                             \
> > +               .ARM.attributes 0 : { *(.ARM.attributes) }
>
> I had to look up what the `0` meant:
> https://sourceware.org/binutils/docs/ld/Output-Section-Attributes.html#Output-Section-Attributes
> mentions it's an "address" and
> https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_chapter/ld_3.html#SEC21
> mentions it as "start" (an address).
> Unless we need those, can we drop them? (Sorry for the resulting churn
> that would cause).  I think the NO_LOAD stuff makes more sense, but
> I'm curious if the kernel checks for that.

NOLOAD means SHT_NOBITS (usually SHF_ALLOC). .ARM.attributes is a
non-SHF_ALLOC section.

An explicit 0 (output section address) is good - GNU ld's internal
linker scripts (ld --verbose output) use 0 for such non-SHF_ALLOC sections.
Without the 0, the section may get a non-zero address, which is not
wrong - but probably does not look well. See https://reviews.llvm.org/D85867 for details.


Reviewed-by: Fangrui Song <maskray@xxxxxxxxxx>

> > +
> >  #define ARM_STUBS_TEXT                                                 \
> >                 *(.gnu.warning)                                         \
> >                 *(.glue_7)                                              \
> > diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
> > index 904c31fa20ed..57fcbf55f913 100644
> > --- a/arch/arm/kernel/vmlinux-xip.lds.S
> > +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> > @@ -150,7 +150,7 @@ SECTIONS
> >         _end = .;
> >
> >         STABS_DEBUG
> > -       ELF_DETAILS
> > +       ARM_DETAILS
> >  }
> >
> >  /*
> > diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> > index bb950c896a67..1d3d3b599635 100644
> > --- a/arch/arm/kernel/vmlinux.lds.S
> > +++ b/arch/arm/kernel/vmlinux.lds.S
> > @@ -149,7 +149,7 @@ SECTIONS
> >         _end = .;
> >
> >         STABS_DEBUG
> > -       ELF_DETAILS
> > +       ARM_DETAILS
> >  }
> >
> >  #ifdef CONFIG_STRICT_KERNEL_RWX
> > --
> > 2.25.1