[PATCH] x86/boot: explicitly place .eh_frame after .rodata

[PATCH] x86/boot: explicitly place .eh_frame after .rodata

[Ilie Halip]

When using GCC as compiler and LLVM's lld as linker, linking
setup.elf fails:
      LD      arch/x86/boot/setup.elf
    ld.lld: error: init sections too big!

This happens because ld.lld has different rules for placing
orphan sections (i.e. sections not mentioned in a linker script)
compared to ld.bfd.

Particularly, in this case, the merged .eh_frame section is
placed before __end_init, which triggers an assert in the script.

Explicitly place this section after .rodata, in accordance with
ld.bfd's behavior.

Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/760
---
 arch/x86/boot/setup.ld | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 0149e41d42c2..4e02eab11b59 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -25,6 +25,7 @@ SECTIONS
 
 	. = ALIGN(16);
 	.rodata		: { *(.rodata*) }
+	.eh_frame	: { *(.eh_frame*) }
 
 	.videocards	: {
 		video_cards = .;
-- 
2.17.1

Re: [PATCH] x86/boot: explicitly place .eh_frame after .rodata

[Nick Desaulniers]

On Mon, Nov 4, 2019 at 1:03 AM Ilie Halip <ilie.halip@gmail.com> wrote:
>
> When using GCC as compiler and LLVM's lld as linker, linking
> setup.elf fails:
>       LD      arch/x86/boot/setup.elf
>     ld.lld: error: init sections too big!
>
> This happens because ld.lld has different rules for placing
> orphan sections (i.e. sections not mentioned in a linker script)
> compared to ld.bfd.
>
> Particularly, in this case, the merged .eh_frame section is
> placed before __end_init, which triggers an assert in the script.
>
> Explicitly place this section after .rodata, in accordance with
> ld.bfd's behavior.
>
> Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/760

Thanks for the patch Ilie! Quoting Fangrui:

"This is related to the orphan placement rule. An orphan section is a
section that is not described by the linker script. The orphan section
placement is not well documented and the rule used by ld.bfd is not
very clear. Being more explicit is the way to go."
https://github.com/ClangBuiltLinux/linux/issues/760#issuecomment-549064237

Looks like Clang doesn't even produce a .eh_frame section.


> ---
>  arch/x86/boot/setup.ld | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> index 0149e41d42c2..4e02eab11b59 100644
> --- a/arch/x86/boot/setup.ld
> +++ b/arch/x86/boot/setup.ld
> @@ -25,6 +25,7 @@ SECTIONS
>
>         . = ALIGN(16);
>         .rodata         : { *(.rodata*) }
> +       .eh_frame       : { *(.eh_frame*) }

The wildcard on the end can be left off; we don't need to glob
different sections with the prefix `.eh_frame`.  Would you mind
sending a V2 with that removed? (I know .rodata and .data in this
linker script globs, but they may actually be putting data in separate
sections which we want to munge back together; certainly for
-fdata-sections).

>
>         .videocards     : {
>                 video_cards = .;
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] x86/boot: explicitly place .eh_frame after .rodata

[Ilie Halip]

> The wildcard on the end can be left off; we don't need to glob
> different sections with the prefix `.eh_frame`.

Sounds good, it doesn't look like any .eh_frame_hdr sections are
being created so it should be fine to remove that wildcard.

I.H.

[PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Ilie Halip]

When using GCC as compiler and LLVM's lld as linker, linking
setup.elf fails:
      LD      arch/x86/boot/setup.elf
    ld.lld: error: init sections too big!

This happens because ld.lld has different rules for placing
orphan sections (i.e. sections not mentioned in a linker script)
compared to ld.bfd.

Particularly, in this case, the merged .eh_frame section is
placed before __end_init, which triggers an assert in the script.

Explicitly place this section after .rodata, in accordance with
ld.bfd's behavior.

Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
Link: https://github.com/ClangBuiltLinux/linux/issues/760
---

Changes in V2:
 * removed wildcard for input sections (.eh_frame* -> .eh_frame)

 arch/x86/boot/setup.ld | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 0149e41d42c2..30ce52635cd0 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -25,6 +25,7 @@ SECTIONS
 
 	. = ALIGN(16);
 	.rodata		: { *(.rodata*) }
+	.eh_frame	: { *(.eh_frame) }
 
 	.videocards	: {
 		video_cards = .;
-- 
2.17.1

Re: [PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Nick Desaulniers]

On Wed, Nov 6, 2019 at 4:06 AM Ilie Halip <ilie.halip@gmail.com> wrote:
>
> When using GCC as compiler and LLVM's lld as linker, linking
> setup.elf fails:
>       LD      arch/x86/boot/setup.elf
>     ld.lld: error: init sections too big!
>
> This happens because ld.lld has different rules for placing
> orphan sections (i.e. sections not mentioned in a linker script)
> compared to ld.bfd.
>
> Particularly, in this case, the merged .eh_frame section is
> placed before __end_init, which triggers an assert in the script.
>
> Explicitly place this section after .rodata, in accordance with
> ld.bfd's behavior.
>
> Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/760

Thanks for following up with a v2.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in V2:
>  * removed wildcard for input sections (.eh_frame* -> .eh_frame)
>
>  arch/x86/boot/setup.ld | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> index 0149e41d42c2..30ce52635cd0 100644
> --- a/arch/x86/boot/setup.ld
> +++ b/arch/x86/boot/setup.ld
> @@ -25,6 +25,7 @@ SECTIONS
>
>         . = ALIGN(16);
>         .rodata         : { *(.rodata*) }
> +       .eh_frame       : { *(.eh_frame) }
>
>         .videocards     : {
>                 video_cards = .;
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Ilie Halip]

Has anyone had a chance to look over this patch?

Thanks,
I.H.

Re: [PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Borislav Petkov]

On Wed, Nov 06, 2019 at 02:06:28PM +0200, Ilie Halip wrote:
> When using GCC as compiler and LLVM's lld as linker, linking
> setup.elf fails:
>       LD      arch/x86/boot/setup.elf
>     ld.lld: error: init sections too big!
> 
> This happens because ld.lld has different rules for placing
> orphan sections (i.e. sections not mentioned in a linker script)
> compared to ld.bfd.
> 
> Particularly, in this case, the merged .eh_frame section is
> placed before __end_init, which triggers an assert in the script.
> 
> Explicitly place this section after .rodata, in accordance with
> ld.bfd's behavior.
> 
> Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> Link: https://github.com/ClangBuiltLinux/linux/issues/760
> ---
> 
> Changes in V2:
>  * removed wildcard for input sections (.eh_frame* -> .eh_frame)
> 
>  arch/x86/boot/setup.ld | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> index 0149e41d42c2..30ce52635cd0 100644
> --- a/arch/x86/boot/setup.ld
> +++ b/arch/x86/boot/setup.ld
> @@ -25,6 +25,7 @@ SECTIONS
>  
>  	. = ALIGN(16);
>  	.rodata		: { *(.rodata*) }
> +	.eh_frame	: { *(.eh_frame) }

The kernel proper linker script does

        /DISCARD/ : {
                *(.eh_frame)
        }

with the .eh_frame section.

Wouldn't that solve your issue too, if you add it to the /DISCARD/
section in that linker script too?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Nick Desaulniers]

On Mon, Nov 18, 2019 at 6:36 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Nov 06, 2019 at 02:06:28PM +0200, Ilie Halip wrote:
> > diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> > index 0149e41d42c2..30ce52635cd0 100644
> > --- a/arch/x86/boot/setup.ld
> > +++ b/arch/x86/boot/setup.ld
> > @@ -25,6 +25,7 @@ SECTIONS
> >
> >       . = ALIGN(16);
> >       .rodata         : { *(.rodata*) }
> > +     .eh_frame       : { *(.eh_frame) }
>
> The kernel proper linker script does
>
>         /DISCARD/ : {
>                 *(.eh_frame)
>         }
>
> with the .eh_frame section.
>
> Wouldn't that solve your issue too, if you add it to the /DISCARD/
> section in that linker script too?

Yep. Looks like:
- arch/x86/kernel/vmlinux.lds.S
- arch/x86/realmode/rm/realmode.lds.S

discard .eh_frame, while
- arch/x86/entry/vdso/vdso-layout.lds.S
- arch/x86/um/vdso/vdso-layout.lds.S

keep it.  I assume then that just vdso code that get linked into
userspace needs to preserve this.  This suggestion would be a
functional change, which is why we pursued the conservative change
preserving it.
https://github.com/ClangBuiltLinux/linux/issues/760#issuecomment-549192213

Ilie, would you mind sending a v3 with Boris' recommendation?

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH V2] x86/boot: explicitly place .eh_frame after .rodata

[Borislav Petkov]

On Mon, Nov 18, 2019 at 09:46:23AM -0800, Nick Desaulniers wrote:
> Yep. Looks like:
> - arch/x86/kernel/vmlinux.lds.S
> - arch/x86/realmode/rm/realmode.lds.S
> 
> discard .eh_frame, while
> - arch/x86/entry/vdso/vdso-layout.lds.S
> - arch/x86/um/vdso/vdso-layout.lds.S
> 
> keep it.  I assume then that just vdso code that get linked into
> userspace needs to preserve this.

Yap, that's what I think too. Lemme add Andy to Cc.

> This suggestion would be a functional change, which is why we pursued
> the conservative change preserving it.

Sure but what would be the purpose of preserving the section, especially
in the early boot code? At least I don't see one. And kernel-proper
kills it...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

[PATCH v3] x86/boot: discard .eh_frame sections

[Ilie Halip]

When using GCC as compiler and LLVM's lld as linker, linking
setup.elf fails:
      LD      arch/x86/boot/setup.elf
    ld.lld: error: init sections too big!

This happens because GCC generates .eh_frame sections for most
of the files in that directory, then ld.lld places the merged
section before __end_init, triggering an assert in the linker
script.

Fix this by discarding the .eh_frame sections, as suggested by
Boris. The kernel proper linker script discards them too.

Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
Link: https://lore.kernel.org/lkml/20191118175223.GM6363@zn.tnic/
Link: https://github.com/ClangBuiltLinux/linux/issues/760
Suggested-by: Borislav Petkov <bp@alien8.de>
---

Changes in V3:
 * discard .eh_frame instead of placing it after .rodata

Changes in V2:
 * removed wildcard for input sections (.eh_frame* -> .eh_frame)

 arch/x86/boot/setup.ld | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 0149e41d42c2..3da1c37c6dd5 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -51,7 +51,10 @@ SECTIONS
 	. = ALIGN(16);
 	_end = .;
 
-	/DISCARD/ : { *(.note*) }
+	/DISCARD/	: {
+		*(.eh_frame)
+		*(.note*)
+	}
 
 	/*
 	 * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
-- 
2.17.1

Re: [PATCH v3] x86/boot: discard .eh_frame sections

[Nick Desaulniers]

On Tue, Nov 26, 2019 at 6:46 AM Ilie Halip <ilie.halip@gmail.com> wrote:
>
> When using GCC as compiler and LLVM's lld as linker, linking
> setup.elf fails:
>       LD      arch/x86/boot/setup.elf
>     ld.lld: error: init sections too big!
>
> This happens because GCC generates .eh_frame sections for most
> of the files in that directory, then ld.lld places the merged
> section before __end_init, triggering an assert in the linker
> script.
>
> Fix this by discarding the .eh_frame sections, as suggested by
> Boris. The kernel proper linker script discards them too.
>
> Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> Link: https://lore.kernel.org/lkml/20191118175223.GM6363@zn.tnic/
> Link: https://github.com/ClangBuiltLinux/linux/issues/760
> Suggested-by: Borislav Petkov <bp@alien8.de>

Ilie, thanks for following up with a v3.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
> Changes in V3:
>  * discard .eh_frame instead of placing it after .rodata
>
> Changes in V2:
>  * removed wildcard for input sections (.eh_frame* -> .eh_frame)
>
>  arch/x86/boot/setup.ld | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> index 0149e41d42c2..3da1c37c6dd5 100644
> --- a/arch/x86/boot/setup.ld
> +++ b/arch/x86/boot/setup.ld
> @@ -51,7 +51,10 @@ SECTIONS
>         . = ALIGN(16);
>         _end = .;
>
> -       /DISCARD/ : { *(.note*) }
> +       /DISCARD/       : {
> +               *(.eh_frame)
> +               *(.note*)
> +       }
>
>         /*
>          * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] x86/boot: discard .eh_frame sections

[Nick Desaulniers]

Bumping for review (I couldn't find if this was merged already in tip/tip)

On Tue, Nov 26, 2019 at 9:16 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Nov 26, 2019 at 6:46 AM Ilie Halip <ilie.halip@gmail.com> wrote:
> >
> > When using GCC as compiler and LLVM's lld as linker, linking
> > setup.elf fails:
> >       LD      arch/x86/boot/setup.elf
> >     ld.lld: error: init sections too big!
> >
> > This happens because GCC generates .eh_frame sections for most
> > of the files in that directory, then ld.lld places the merged
> > section before __end_init, triggering an assert in the linker
> > script.
> >
> > Fix this by discarding the .eh_frame sections, as suggested by
> > Boris. The kernel proper linker script discards them too.
> >
> > Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
> > Link: https://lore.kernel.org/lkml/20191118175223.GM6363@zn.tnic/
> > Link: https://github.com/ClangBuiltLinux/linux/issues/760
> > Suggested-by: Borislav Petkov <bp@alien8.de>
>
> Ilie, thanks for following up with a v3.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> > ---
> >
> > Changes in V3:
> >  * discard .eh_frame instead of placing it after .rodata
> >
> > Changes in V2:
> >  * removed wildcard for input sections (.eh_frame* -> .eh_frame)
> >
> >  arch/x86/boot/setup.ld | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> > index 0149e41d42c2..3da1c37c6dd5 100644
> > --- a/arch/x86/boot/setup.ld
> > +++ b/arch/x86/boot/setup.ld
> > @@ -51,7 +51,10 @@ SECTIONS
> >         . = ALIGN(16);
> >         _end = .;
> >
> > -       /DISCARD/ : { *(.note*) }
> > +       /DISCARD/       : {
> > +               *(.eh_frame)
> > +               *(.note*)
> > +       }
> >
> >         /*
> >          * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility:
> > --
> > 2.17.1
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Thanks,
~Nick Desaulniers

[tip: x86/boot] x86/boot: Discard .eh_frame sections

[tip-bot2 for Ilie Halip]

The following commit has been merged into the x86/boot branch of tip:

Commit-ID:     163159aad74d3763b350861b879b41e8f64121fc
Gitweb:        https://git.kernel.org/tip/163159aad74d3763b350861b879b41e8f64121fc
Author:        Ilie Halip <ilie.halip@gmail.com>
AuthorDate:    Tue, 26 Nov 2019 16:45:44 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Fri, 13 Dec 2019 11:45:59 +01:00

x86/boot: Discard .eh_frame sections

When using GCC as compiler and LLVM's lld as linker, linking setup.elf
fails:

   LD      arch/x86/boot/setup.elf
  ld.lld: error: init sections too big!

This happens because GCC generates .eh_frame sections for most of the
files in that directory, then ld.lld places the merged section before
__end_init, triggering an assert in the linker script.

Fix this by discarding the .eh_frame sections, as suggested by Boris.
The kernel proper linker script discards them too.

 [ bp: Going back in history, 64-bit kernel proper has been discarding
   .eh_frame since 2002:

    commit acca80acefe20420e69561cf55be64f16c34ea97
    Author: Andi Kleen <ak@muc.de>
    Date:   Tue Oct 29 23:54:35 2002 -0800

      [PATCH] x86-64 updates for 2.5.44

      ...

    - Remove the .eh_frame on linking. This saves several hundred KB in the
      bzImage
 ]

Suggested-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Ilie Halip <ilie.halip@gmail.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com
Cc: Andy Lutomirski <luto@kernel.org>
Cc: clang-built-linux@googlegroups.com
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: x86-ml <x86@kernel.org>
Link: https://lore.kernel.org/lkml/20191118175223.GM6363@zn.tnic/
Link: https://github.com/ClangBuiltLinux/linux/issues/760
Link: https://lkml.kernel.org/r/20191126144545.19354-1-ilie.halip@gmail.com
---
 arch/x86/boot/setup.ld | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 0149e41..3da1c37 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -51,7 +51,10 @@ SECTIONS
 	. = ALIGN(16);
 	_end = .;
 
-	/DISCARD/ : { *(.note*) }
+	/DISCARD/	: {
+		*(.eh_frame)
+		*(.note*)
+	}
 
 	/*
 	 * The ASSERT() sink to . is intentional, for binutils 2.14 compatibility: