2020-06-22 22:09:50

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] x86/boot: Warn on orphan section placement

On 2020-06-22, 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.
>
>Add the common debugging sections. Discard the unused note, rel, plt,
>dyn, and hash sections that are not needed in the compressed vmlinux.
>Disable .eh_frame generation in the linker and enable orphan section
>warnings.
>
>Signed-off-by: Kees Cook <[email protected]>
>---
> arch/x86/boot/compressed/Makefile | 3 ++-
> arch/x86/boot/compressed/vmlinux.lds.S | 11 +++++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>index 7619742f91c9..646720a05f89 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -48,6 +48,7 @@ GCOV_PROFILE := n
> UBSAN_SANITIZE :=n
>
> KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
>+KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)
> # Compressed kernel should be built as PIE since it may be loaded at any
> # address by the bootloader.
> ifeq ($(CONFIG_X86_32),y)
>@@ -59,7 +60,7 @@ else
> KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
> && echo "-z noreloc-overflow -pie --no-dynamic-linker")
> endif
>-LDFLAGS_vmlinux := -T
>+LDFLAGS_vmlinux := --orphan-handling=warn -T
>
> hostprogs := mkpiggy
> HOST_EXTRACFLAGS += -I$(srctree)/tools/include
>diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>index 8f1025d1f681..6fe3ecdfd685 100644
>--- a/arch/x86/boot/compressed/vmlinux.lds.S
>+++ b/arch/x86/boot/compressed/vmlinux.lds.S
>@@ -75,5 +75,16 @@ SECTIONS
> . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */
> _end = .;
>
>+ STABS_DEBUG
>+ DWARF_DEBUG
>+
> DISCARDS
>+ /DISCARD/ : {
>+ *(.note.*)
>+ *(.rela.*) *(.rela_*)
>+ *(.rel.*) *(.rel_*)
>+ *(.plt) *(.plt.*)
>+ *(.dyn*)
>+ *(.hash) *(.gnu.hash)
>+ }
> }
>--
>2.25.1

LLD may report warnings for 3 synthetic sections if they are orphans:

ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab'
ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab'

Are they described?


2020-06-22 22:40:23

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] x86/boot: Warn on orphan section placement

On Mon, Jun 22, 2020 at 03:06:28PM -0700, Fangrui Song wrote:
> On 2020-06-22, 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.
> >
> > Add the common debugging sections. Discard the unused note, rel, plt,
> > dyn, and hash sections that are not needed in the compressed vmlinux.
> > Disable .eh_frame generation in the linker and enable orphan section
> > warnings.
> >
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > arch/x86/boot/compressed/Makefile | 3 ++-
> > arch/x86/boot/compressed/vmlinux.lds.S | 11 +++++++++++
> > 2 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > index 7619742f91c9..646720a05f89 100644
> > --- a/arch/x86/boot/compressed/Makefile
> > +++ b/arch/x86/boot/compressed/Makefile
> > @@ -48,6 +48,7 @@ GCOV_PROFILE := n
> > UBSAN_SANITIZE :=n
> >
> > KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
> > +KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)
> > # Compressed kernel should be built as PIE since it may be loaded at any
> > # address by the bootloader.
> > ifeq ($(CONFIG_X86_32),y)
> > @@ -59,7 +60,7 @@ else
> > KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
> > && echo "-z noreloc-overflow -pie --no-dynamic-linker")
> > endif
> > -LDFLAGS_vmlinux := -T
> > +LDFLAGS_vmlinux := --orphan-handling=warn -T
> >
> > hostprogs := mkpiggy
> > HOST_EXTRACFLAGS += -I$(srctree)/tools/include
> > diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> > index 8f1025d1f681..6fe3ecdfd685 100644
> > --- a/arch/x86/boot/compressed/vmlinux.lds.S
> > +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> > @@ -75,5 +75,16 @@ SECTIONS
> > . = ALIGN(PAGE_SIZE); /* keep ZO size page aligned */
> > _end = .;
> >
> > + STABS_DEBUG
> > + DWARF_DEBUG
> > +
> > DISCARDS
> > + /DISCARD/ : {
> > + *(.note.*)
> > + *(.rela.*) *(.rela_*)
> > + *(.rel.*) *(.rel_*)
> > + *(.plt) *(.plt.*)
> > + *(.dyn*)
> > + *(.hash) *(.gnu.hash)
> > + }
> > }
> > --
> > 2.25.1
>
> LLD may report warnings for 3 synthetic sections if they are orphans:
>
> ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab'
> ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
> ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab'
>
> Are they described?

Ah, hm. I see gcc is just silent about these. It looks like both regular
and debug kernels end up with those sections for both GCC and Clang. How
would you expect them to be described?

--
Kees Cook

2020-06-22 22:46:23

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] x86/boot: Warn on orphan section placement

On Mon, Jun 22, 2020 at 03:06:28PM -0700, Fangrui Song wrote:
> LLD may report warnings for 3 synthetic sections if they are orphans:
>
> ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab'
> ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
> ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab'
>
> Are they described?

Perhaps:

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index db600ef218d7..57e9c142e401 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -792,6 +792,9 @@
.stab.exclstr 0 : { *(.stab.exclstr) } \
.stab.index 0 : { *(.stab.index) } \
.stab.indexstr 0 : { *(.stab.indexstr) } \
+ .symtab 0 : { *(.symtab) } \
+ .strtab 0 : { *(.strtab) } \
+ .shstrtab 0 : { *(.shstrtab) } \
.comment 0 : { *(.comment) }

#ifdef CONFIG_GENERIC_BUG

--
Kees Cook

2020-06-22 22:53:55

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] x86/boot: Warn on orphan section placement

On 2020-06-22, Kees Cook wrote:
>On Mon, Jun 22, 2020 at 03:06:28PM -0700, Fangrui Song wrote:
>> LLD may report warnings for 3 synthetic sections if they are orphans:
>>
>> ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab'
>> ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
>> ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab'
>>
>> Are they described?
>
>Perhaps:
>
>diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
>index db600ef218d7..57e9c142e401 100644
>--- a/include/asm-generic/vmlinux.lds.h
>+++ b/include/asm-generic/vmlinux.lds.h
>@@ -792,6 +792,9 @@
> .stab.exclstr 0 : { *(.stab.exclstr) } \
> .stab.index 0 : { *(.stab.index) } \
> .stab.indexstr 0 : { *(.stab.indexstr) } \
>+ .symtab 0 : { *(.symtab) } \
>+ .strtab 0 : { *(.strtab) } \
>+ .shstrtab 0 : { *(.shstrtab) } \
> .comment 0 : { *(.comment) }
>
> #ifdef CONFIG_GENERIC_BUG

This LGTM. Nit: .comment before .symtab is a more common order.

Reviewed-by: Fangrui Song <[email protected]>

2020-06-22 23:05:13

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] x86/boot: Warn on orphan section placement

On Mon, Jun 22, 2020 at 03:49:28PM -0700, Fangrui Song wrote:
> On 2020-06-22, Kees Cook wrote:
> > On Mon, Jun 22, 2020 at 03:06:28PM -0700, Fangrui Song wrote:
> > > LLD may report warnings for 3 synthetic sections if they are orphans:
> > >
> > > ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab'
> > > ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab'
> > > ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab'
> > >
> > > Are they described?
> >
> > Perhaps:
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index db600ef218d7..57e9c142e401 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -792,6 +792,9 @@
> > .stab.exclstr 0 : { *(.stab.exclstr) } \
> > .stab.index 0 : { *(.stab.index) } \
> > .stab.indexstr 0 : { *(.stab.indexstr) } \
> > + .symtab 0 : { *(.symtab) } \
> > + .strtab 0 : { *(.strtab) } \
> > + .shstrtab 0 : { *(.shstrtab) } \
> > .comment 0 : { *(.comment) }
> >
> > #ifdef CONFIG_GENERIC_BUG
>
> This LGTM. Nit: .comment before .symtab is a more common order.

Adjusted.

> Reviewed-by: Fangrui Song <[email protected]>

Thanks!

--
Kees Cook