[PATCH v2 2/2] arm64/build: Warn on orphan section placement

[Nick Desaulniers]

On Tue, Jun 23, 2020 at 7:59 AM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Tue, 23 Jun 2020 at 16:52, Will Deacon <will at kernel.org> wrote:
> >
> > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook wrote:
> > > We don't want to depend on the linker's orphan section placement
> > > heuristics as these can vary between linkers, and may change between
> > > versions. All sections need to be explicitly named in the linker
> > > script.
> > >
> > > Explicitly include debug sections when they're present. Add .eh_frame*
> > > to discard as it seems that these are still generated even though
> > > -fno-asynchronous-unwind-tables is being specified. Add .plt and
> > > .data.rel.ro to discards as they are not actually used. Add .got.plt
> > > to the image as it does appear to be mapped near .data. Finally enable
> > > orphan section warnings.
> >
> > Can you elaborate a bit on what .got.plt is being used for, please? I
> > wonder if there's an interaction with an erratum workaround in the linker
> > or something.
> >
>
> .got.plt is not used at all, but it has three magic entries at the
> start that the dynamic linker uses for lazy dispatch, so it turns up
> as a non-empty section of 0x18 bytes.

Interesting; is there a way to dump those entries? `--dynamic-reloc`
flag to objdump? (I suspect the answer might be hexdump...)

> We should be able to discard it afaict, but given that it does not
> actually take up any space, it doesn't really matter either way.

True, but I would prefer to explicitly discard it if we know we're not
using it, that way something explicitly breaks if someone tries to
make use of it in the future.  Then we can consider not discarding it,
only if necessary.  Modules on arm64 use .got.plt, IIRC? But they have
their own linker script so irrelevant I guess.

> > > --- a/arch/arm64/kernel/vmlinux.lds.S
> > > +++ b/arch/arm64/kernel/vmlinux.lds.S
> > > @@ -94,7 +94,8 @@ SECTIONS
> > >       /DISCARD/ : {
> > >               *(.interp .dynamic)
> > >               *(.dynsym .dynstr .hash .gnu.hash)
> > > -             *(.eh_frame)
> > > +             *(.plt) *(.data.rel.ro)
> > > +             *(.eh_frame) *(.init.eh_frame)
> >
> > Do we need to include .eh_frame_hdr here too?
> >
>
> It would be better to build with -fno-unwind-tables, in which case
> these sections should not even exist.

Interesting, so we have -fno-asynchronous-unwind-tables and
-fno-unwind-tables.  Is your suggestion for -fno-unwind-tables a
global KBUILD_CFLAG (vs limited to a particular arch)?  Interestingly,
there a few users of -fasynchronous-unwind-tables in the kernel.
vdso's make sense, I think, less sure about the rest.

-- 
Thanks,
~Nick Desaulniers