[PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Dmitry Golovin]

LLD by default disallows relocations in read-only segments. For a
relocatable kernel, we pass -z notext to the linker to explicitly
allow relocations. This behavior is the default for BFD.

Link: https://github.com/ClangBuiltLinux/linux/issues/579
Signed-off-by: Dmitry Golovin <dima@golovin.in>
---
 arch/x86/boot/compressed/Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 5f7c262bcc99..7214751e1671 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -57,6 +57,9 @@ else
 KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
 	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
 endif
+ifeq ($(CONFIG_RELOCATABLE), y)
+KBUILD_LDFLAGS += -z notext
+endif
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy
-- 
2.25.1

Re: [PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Nathan Chancellor]

On Fri, May 01, 2020 at 08:42:13AM +0000, Dmitry Golovin wrote:
> LLD by default disallows relocations in read-only segments. For a
> relocatable kernel, we pass -z notext to the linker to explicitly
> allow relocations. This behavior is the default for BFD.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/579
> Signed-off-by: Dmitry Golovin <dima@golovin.in>

I was able to link a Clang built i386_defconfig kernel with ld.lld and
boot it in QEMU 5.0 after this change. A GCC built kernel links still
with ld.bfd and also boots in QEMU successfully. x86_64_defconfig with
both compilers and their respective linkers did not regress.

Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  arch/x86/boot/compressed/Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 5f7c262bcc99..7214751e1671 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -57,6 +57,9 @@ else
>  KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
>  	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
>  endif
> +ifeq ($(CONFIG_RELOCATABLE), y)
> +KBUILD_LDFLAGS += -z notext
> +endif
>  LDFLAGS_vmlinux := -T
>  
>  hostprogs	:= mkpiggy
> -- 
> 2.25.1
> 

Re: [PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Borislav Petkov]

On Fri, May 01, 2020 at 08:42:13AM +0000, Dmitry Golovin wrote:
> LLD by default disallows relocations in read-only segments. For a

I need more info here about which segment is read-only?

Is this something LLD does by default or what's happening?

Because my BFD-linked vmlinux has:

Program Headers:
  Type           Offset             VirtAddr           PhysAddr
                 FileSiz            MemSiz              Flags  Align
  LOAD           0x0000000000001000 0x0000000000000000 0x0000000000000000
                 0x000000000070fa28 0x0000000000726b00  RWE    0x1000
  LOAD           0x0000000000000000 0x0000000000727000 0x0000000000727000
                 0x0000000000000000 0x0000000000007000  RW     0x1000
  DYNAMIC        0x00000000007108f8 0x000000000070f8f8 0x000000000070f8f8
                 0x0000000000000130 0x0000000000000130  RW     0x8
  GNU_STACK      0x0000000000000000 0x0000000000000000 0x0000000000000000
                 0x0000000000000000 0x0000000000000000  RWE    0x10

so what's up?

> relocatable kernel, we pass -z notext to the linker to explicitly
> allow relocations. This behavior is the default for BFD.

Or are you saying that ld.bfd makes the text segment by default RW while
ld.lld makes it read-only like the elf manpage says:

"p_flags
              This member holds a bit mask of flags relevant to the segment:

              PF_X   An executable segment.
              PF_W   A writable segment.
              PF_R   A readable segment.

              A text segment commonly has the flags PF_X and PF_R."

IOW, don't be afraid to be more verbose in the commit message. :)

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Fangrui Song]

On 2020-05-16, Dmitry Golovin wrote:
>15.05.2020, 21:50, "Borislav Petkov" <bp@alien8.de>:
>>
>> I need more info here about which segment is read-only?
>>
>> Is this something LLD does by default or what's happening?
>>
>
>Probably should have quoted the original error message:
>
>    ld.lld: error: can't create dynamic relocation R_386_32 against
>    symbol: _bss in readonly segment; recompile object files with -fPIC
>    or pass '-Wl,-z,notext' to allow text relocations in the output

Do we know where do these R_386_32 come from?

When linking in -shared mode, the linker assumes the image is a shared
object and has undetermined image base at runtime. An absolute
relocation needs a text relocation (a relocation against a readonly
segment).

When neither -z notext nor -z text is specified, GNU ld is in an
indefinite state where it will enable text relocations (DT_TEXTREL
DF_TEXTREL) on demand. It is not considered a good practice for
userspace applications to do this.

Of course the kernel is different....... I know little about the kernel,
but if there is a way to make the sections containing R_386_32
relocations writable (SHF_WRITE), that will be a better solution to me.
In LLD, -z notext is like making every section SHF_WRITE.

>>
>> IOW, don't be afraid to be more verbose in the commit message. :)
>>
>
>Tried both BFD and LLD for linking to understand the difference more and
>rewrite the commit message, and came to the conclusion that the patch is
>wrong. I will submit v2 when I figure out the correct solution.
>
>Regards,
>Dmitry
>
>-- 
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/602331589572661%40mail.yandex.ru.

Re: [PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Arvind Sankar]

On Sun, May 17, 2020 at 12:44:29PM -0700, Fangrui Song wrote:
> On 2020-05-16, Dmitry Golovin wrote:
> >15.05.2020, 21:50, "Borislav Petkov" <bp@alien8.de>:
> >>
> >> I need more info here about which segment is read-only?
> >>
> >> Is this something LLD does by default or what's happening?
> >>
> >
> >Probably should have quoted the original error message:
> >
> >    ld.lld: error: can't create dynamic relocation R_386_32 against
> >    symbol: _bss in readonly segment; recompile object files with -fPIC
> >    or pass '-Wl,-z,notext' to allow text relocations in the output
> 
> Do we know where do these R_386_32 come from?
> 
> When linking in -shared mode, the linker assumes the image is a shared
> object and has undetermined image base at runtime. An absolute
> relocation needs a text relocation (a relocation against a readonly
> segment).
> 
> When neither -z notext nor -z text is specified, GNU ld is in an
> indefinite state where it will enable text relocations (DT_TEXTREL
> DF_TEXTREL) on demand. It is not considered a good practice for
> userspace applications to do this.
> 
> Of course the kernel is different....... I know little about the kernel,
> but if there is a way to make the sections containing R_386_32
> relocations writable (SHF_WRITE), that will be a better solution to me.
> In LLD, -z notext is like making every section SHF_WRITE.

The assembly files head_32.S and head_64.S in arch/x86/boot/compressed
create bogus relocations in .head.text and .text.

This is the source of the common warning when using the bfd linker, for
eg on 64-bit:
  LD      arch/x86/boot/compressed/vmlinux
  ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
  ld: warning: creating a DT_TEXTREL in object

These relocations are "bogus", i.e. they're unwanted and the kernel
won't actually boot if anything were to perform those relocations before
running it. They're also the cause of the 64-bit kernel requiring linker
support for the -z noreloc-overflow option to link it as PIE.

From arch/x86/boot/compressed/Makefile:

# To build 64-bit compressed kernel as PIE, we disable relocation
# overflow check to avoid relocation overflow error with a new linker
# command-line option, -z noreloc-overflow.
KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z
noreloc-overflow" \
        && echo "-z noreloc-overflow -pie --no-dynamic-linker")

The relocations come from code like
	leal    gdt(%ebp), %eax
which should really be
	leal	(gdt-startup_32)(%ebp), %eax
to be technically correct.

I've played around with fixing the head code to avoid creating the
relocations in the first place, but never got around to submitting
anything: if there is interest in this, I can polish it up and send it
around.

Thanks.

Re: [PATCH] x86/boot: allow a relocatable kernel to be linked with lld

[Nick Desaulniers]

On Sun, May 17, 2020 at 1:25 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Sun, May 17, 2020 at 12:44:29PM -0700, Fangrui Song wrote:
> > On 2020-05-16, Dmitry Golovin wrote:
> > >15.05.2020, 21:50, "Borislav Petkov" <bp@alien8.de>:
> > >>
> > >> I need more info here about which segment is read-only?
> > >>
> > >> Is this something LLD does by default or what's happening?
> > >>
> > >
> > >Probably should have quoted the original error message:
> > >
> > >    ld.lld: error: can't create dynamic relocation R_386_32 against
> > >    symbol: _bss in readonly segment; recompile object files with -fPIC
> > >    or pass '-Wl,-z,notext' to allow text relocations in the output
> >
> > Do we know where do these R_386_32 come from?
> >
> > When linking in -shared mode, the linker assumes the image is a shared
> > object and has undetermined image base at runtime. An absolute
> > relocation needs a text relocation (a relocation against a readonly
> > segment).
> >
> > When neither -z notext nor -z text is specified, GNU ld is in an
> > indefinite state where it will enable text relocations (DT_TEXTREL
> > DF_TEXTREL) on demand. It is not considered a good practice for
> > userspace applications to do this.
> >
> > Of course the kernel is different....... I know little about the kernel,
> > but if there is a way to make the sections containing R_386_32
> > relocations writable (SHF_WRITE), that will be a better solution to me.
> > In LLD, -z notext is like making every section SHF_WRITE.
>
> The assembly files head_32.S and head_64.S in arch/x86/boot/compressed
> create bogus relocations in .head.text and .text.
>
> This is the source of the common warning when using the bfd linker, for
> eg on 64-bit:
>   LD      arch/x86/boot/compressed/vmlinux
>   ld: arch/x86/boot/compressed/head_64.o: warning: relocation in read-only section `.head.text'
>   ld: warning: creating a DT_TEXTREL in object
>
> These relocations are "bogus", i.e. they're unwanted and the kernel
> won't actually boot if anything were to perform those relocations before
> running it. They're also the cause of the 64-bit kernel requiring linker
> support for the -z noreloc-overflow option to link it as PIE.
>
> From arch/x86/boot/compressed/Makefile:
>
> # To build 64-bit compressed kernel as PIE, we disable relocation
> # overflow check to avoid relocation overflow error with a new linker
> # command-line option, -z noreloc-overflow.
> KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z
> noreloc-overflow" \
>         && echo "-z noreloc-overflow -pie --no-dynamic-linker")
>
> The relocations come from code like
>         leal    gdt(%ebp), %eax
> which should really be
>         leal    (gdt-startup_32)(%ebp), %eax
> to be technically correct.
>
> I've played around with fixing the head code to avoid creating the
> relocations in the first place, but never got around to submitting
> anything: if there is interest in this, I can polish it up and send it
> around.

We'd be happy to help test and review. :)
-- 
Thanks,
~Nick Desaulniers

[PATCH 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Arvind Sankar]

The compressed kernel currently contains bogus runtime relocations in
the startup code in head_{32,64}.S, which are generated by the linker,
but must not actually be processed at runtime.

This generates warnings when linking with the BFD linker, and errors
with LLD, which defaults to erroring on runtime relocations in read-only
sections. It also requires the -z noreloc-overflow hack for the 64-bit
kernel, which prevents us from linking it as -pie on an older BFD linker
(<= 2.26) or on LLD, because the locations that are to be apparently
relocated are only 32-bits in size and so cannot normally have
R_X86_64_RELATIVE relocations.

This series aims to get rid of these relocations. It is based on
efi/next (efi-changes-for-v5.8), where the latest patches touch the
head code to eliminate the global offset table.

The first patch is an independent fix for LLD, to avoid an orphan
section in arch/x86/boot/setup.elf [0].

The second patch gets rid of almost all the relocations. It uses
standard PIC addressing technique for 32-bit, i.e. loading a register
with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
references to access variables. For 64-bit, there is 32-bit code that
cannot use RIP-relative addressing, and also cannot use the 32-bit
method, since GOTOFF references are 64-bit only. This is instead handled
using a macro to replace a reference like gdt with (gdt-startup_32)
instead. The assembler will generate a PC32 relocation entry, with
addend set to (.-startup_32), and these will be replaced with constants
at link time. This works as long as all the code using such references
lives in the same section as startup_32, i.e. in .head.text.

The third patch addresses a remaining issue with the BFD linker, which
insists on generating runtime relocations for absolute symbols. We use
z_input_len and z_output_len, defined in the generated piggy.S file, as
symbols whose absolute "addresses" are actually the size of the
compressed payload and the size of the decompressed kernel image
respectively. LLD does not generate relocations for these two symbols,
but the BFD linker does. To get around this, piggy.S is extended to also
define two u32 variables (in .rodata) with the lengths, and the head
code is modified to use those instead of the symbol addresses.

An alternative way to handle z_input_len/z_output_len would be to just
include piggy.S in head_{32,64}.S instead of as a separate object file,
since the GNU assembler doesn't generate relocations for symbols set to
constants.

The last patch adds a check in the linker script to ensure that no
runtime relocations get reintroduced. Since the GOT has been eliminated
as well, the compressed kernel has no runtime relocations whatsoever any
more.

[0] https://lore.kernel.org/lkml/20200521152459.558081-1-nivedita@alum.mit.edu/

Arvind Sankar (4):
  x86/boot: Add .text.startup to setup.ld
  x86/boot: Remove runtime relocations from .head.text code
  x86/boot: Remove runtime relocations from head_{32,64}.S
  x86/boot: Check that there are no runtime relocations

 arch/x86/boot/compressed/Makefile      | 36 +---------
 arch/x86/boot/compressed/head_32.S     | 59 +++++++--------
 arch/x86/boot/compressed/head_64.S     | 99 +++++++++++++++-----------
 arch/x86/boot/compressed/mkpiggy.c     |  6 ++
 arch/x86/boot/compressed/vmlinux.lds.S | 11 +++
 arch/x86/boot/setup.ld                 |  2 +-
 6 files changed, 109 insertions(+), 104 deletions(-)

-- 
2.26.2

[PATCH 1/4] x86/boot: Add .text.startup to setup.ld

[Arvind Sankar]

gcc puts the main function into .text.startup when compiled with -Os (or
-O2). This results in arch/x86/boot/main.c having a .text.startup
section which is currently not included explicitly in the linker script
setup.ld in the same directory.

The BFD linker places this orphan section immediately after .text, so
this still works. However, LLD git, since [1], is choosing to place it
immediately after the .bstext section instead (this is the first code
section). This plays havoc with the section layout that setup.elf
requires to create the setup header, for eg on 64-bit:

    LD      arch/x86/boot/setup.elf
  ld.lld: error: section .text.startup file range overlaps with .header
  >>> .text.startup range is [0x200040, 0x2001FE]
  >>> .header range is [0x2001EF, 0x20026B]

  ld.lld: error: section .header file range overlaps with .bsdata
  >>> .header range is [0x2001EF, 0x20026B]
  >>> .bsdata range is [0x2001FF, 0x200398]

  ld.lld: error: section .bsdata file range overlaps with .entrytext
  >>> .bsdata range is [0x2001FF, 0x200398]
  >>> .entrytext range is [0x20026C, 0x2002D3]

  ld.lld: error: section .text.startup virtual address range overlaps
  with .header
  >>> .text.startup range is [0x40, 0x1FE]
  >>> .header range is [0x1EF, 0x26B]

  ld.lld: error: section .header virtual address range overlaps with
  .bsdata
  >>> .header range is [0x1EF, 0x26B]
  >>> .bsdata range is [0x1FF, 0x398]

  ld.lld: error: section .bsdata virtual address range overlaps with
  .entrytext
  >>> .bsdata range is [0x1FF, 0x398]
  >>> .entrytext range is [0x26C, 0x2D3]

  ld.lld: error: section .text.startup load address range overlaps with
  .header
  >>> .text.startup range is [0x40, 0x1FE]
  >>> .header range is [0x1EF, 0x26B]

  ld.lld: error: section .header load address range overlaps with
  .bsdata
  >>> .header range is [0x1EF, 0x26B]
  >>> .bsdata range is [0x1FF, 0x398]

  ld.lld: error: section .bsdata load address range overlaps with
  .entrytext
  >>> .bsdata range is [0x1FF, 0x398]
  >>> .entrytext range is [0x26C, 0x2D3]

Explicitly pull .text.startup into the .text output section to avoid
this.

[1] https://reviews.llvm.org/D75225

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
Reviewed-by: Fangrui Song <maskray@google.com>
---
 arch/x86/boot/setup.ld | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 24c95522f231..ed60abcdb089 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -20,7 +20,7 @@ SECTIONS
 	.initdata	: { *(.initdata) }
 	__end_init = .;
 
-	.text		: { *(.text) }
+	.text		: { *(.text.startup) *(.text) }
 	.text32		: { *(.text32) }
 
 	. = ALIGN(16);
-- 
2.26.2

[PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code

[Arvind Sankar]

The assembly code in head_{32,64}.S, while meant to be
position-independent, generates run-time relocations because it uses
instructions such as
	leal	gdt(%edx), %eax
which make the assembler and linker think that the code is using %edx as
an index into gdt, and hence gdt needs to be relocated to its run-time
address.

With the BFD linker, this generates a warning during the build:
  LD      arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
ld: warning: creating a DT_TEXTREL in object

With lld, Dmitry Golovin reports that this results in a link-time error
with default options (i.e. unless -z notext is explicitly passed):
  LD      arch/x86/boot/compressed/vmlinux
ld.lld: error: can't create dynamic relocation R_386_32 against local
symbol in readonly segment; recompile object files with -fPIC or pass
'-Wl,-z,notext' to allow text relocations in the output

Start fixing this by removing relocations from .head.text:
- On 32-bit, use a base register that holds the address of the GOT and
  reference symbol addresses using @GOTOFF, i.e.
	leal	gdt@GOTOFF(%edx), %eax
- On 64-bit, most of the code can (and already does) use %rip-relative
  addressing, however the .code32 bits can't, and the 64-bit code also
  needs to reference symbol addresses as they will be after moving the
  compressed kernel to the end of the decompression buffer.
  For these cases, reference the symbols as an offset to startup_32 to
  avoid creating relocations, i.e.
  	leal	(gdt-startup_32)(%bp), %eax
  This only works in .head.text as the subtraction cannot be represented
  as a PC-relative relocation unless startup_32 is in the same section
  as the code. Move efi32_pe_entry into .head.text so that it can use
  the same method to avoid relocations.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/head_32.S | 40 +++++++------
 arch/x86/boot/compressed/head_64.S | 95 ++++++++++++++++++------------
 2 files changed, 77 insertions(+), 58 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index dfa4131c65df..66657bb99aae 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -73,10 +73,10 @@ SYM_FUNC_START(startup_32)
 	leal	(BP_scratch+4)(%esi), %esp
 	call	1f
 1:	popl	%edx
-	subl	$1b, %edx
+	addl	$_GLOBAL_OFFSET_TABLE_+(.-1b), %edx
 
 	/* Load new GDT */
-	leal	gdt(%edx), %eax
+	leal	gdt@GOTOFF(%edx), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
@@ -89,14 +89,16 @@ SYM_FUNC_START(startup_32)
 	movl	%eax, %ss
 
 /*
- * %edx contains the address we are loaded at by the boot loader and %ebx
- * contains the address where we should move the kernel image temporarily
- * for safe in-place decompression. %ebp contains the address that the kernel
- * will be decompressed to.
+ * %edx contains the address we are loaded at by the boot loader (plus the
+ * offset to the GOT).  The below code calculates %ebx to be the address where
+ * we should move the kernel image temporarily for safe in-place decompression
+ * (again, plus the offset to the GOT).
+ *
+ * %ebp is calculated to be the address that the kernel will be decompressed to.
  */
 
 #ifdef CONFIG_RELOCATABLE
-	movl	%edx, %ebx
+	leal	startup_32@GOTOFF(%edx), %ebx
 
 #ifdef CONFIG_EFI_STUB
 /*
@@ -107,7 +109,7 @@ SYM_FUNC_START(startup_32)
  *	image_offset = startup_32 - image_base
  * Otherwise image_offset will be zero and has no effect on the calculations.
  */
-	subl    image_offset(%edx), %ebx
+	subl    image_offset@GOTOFF(%edx), %ebx
 #endif
 
 	movl	BP_kernel_alignment(%esi), %eax
@@ -124,10 +126,10 @@ SYM_FUNC_START(startup_32)
 	movl	%ebx, %ebp	// Save the output address for later
 	/* Target address to relocate to for decompression */
 	addl    BP_init_size(%esi), %ebx
-	subl    $_end, %ebx
+	subl    $_end@GOTOFF, %ebx
 
 	/* Set up the stack */
-	leal	boot_stack_end(%ebx), %esp
+	leal	boot_stack_end@GOTOFF(%ebx), %esp
 
 	/* Zero EFLAGS */
 	pushl	$0
@@ -138,8 +140,8 @@ SYM_FUNC_START(startup_32)
  * where decompression in place becomes safe.
  */
 	pushl	%esi
-	leal	(_bss-4)(%edx), %esi
-	leal	(_bss-4)(%ebx), %edi
+	leal	(_bss@GOTOFF-4)(%edx), %esi
+	leal	(_bss@GOTOFF-4)(%ebx), %edi
 	movl	$(_bss - startup_32), %ecx
 	shrl	$2, %ecx
 	std
@@ -152,14 +154,14 @@ SYM_FUNC_START(startup_32)
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
 	 * to the new copy of the GDT.
 	 */
-	leal	gdt(%ebx), %eax
+	leal	gdt@GOTOFF(%ebx), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
 /*
  * Jump to the relocated address.
  */
-	leal	.Lrelocated(%ebx), %eax
+	leal	.Lrelocated@GOTOFF(%ebx), %eax
 	jmp	*%eax
 SYM_FUNC_END(startup_32)
 
@@ -169,7 +171,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
 	add	$0x4, %esp
 	movl	8(%esp), %esi	/* save boot_params pointer */
 	call	efi_main
-	leal	startup_32(%eax), %eax
+	/* efi_main returns the possibly relocated address of startup_32 */
 	jmp	*%eax
 SYM_FUNC_END(efi32_stub_entry)
 SYM_FUNC_END_ALIAS(efi_stub_entry)
@@ -182,8 +184,8 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
  * Clear BSS (stack is currently empty)
  */
 	xorl	%eax, %eax
-	leal	_bss(%ebx), %edi
-	leal	_ebss(%ebx), %ecx
+	leal	_bss@GOTOFF(%ebx), %edi
+	leal	_ebss@GOTOFF(%ebx), %ecx
 	subl	%edi, %ecx
 	shrl	$2, %ecx
 	rep	stosl
@@ -197,9 +199,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushl	%ebp		/* output address */
 
 	pushl	$z_input_len	/* input_len */
-	leal	input_data(%ebx), %eax
+	leal	input_data@GOTOFF(%ebx), %eax
 	pushl	%eax		/* input_data */
-	leal	boot_heap(%ebx), %eax
+	leal	boot_heap@GOTOFF(%ebx), %eax
 	pushl	%eax		/* heap area */
 	pushl	%esi		/* real mode pointer */
 	call	extract_kernel	/* returns kernel location in %eax */
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 706fbf6eef53..f6ba32cd5702 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -42,6 +42,23 @@
 	.hidden _ebss
 
 	__HEAD
+
+/*
+ * This macro gives the link address of X. It's the same as X, since startup_32
+ * has link address 0, but defining it this way tells the assembler/linker that
+ * we want the link address, and not the run-time address of X. This prevents
+ * the linker from creating a run-time relocation entry for this reference.
+ *
+ * The macro should be used as a displacement with a base register containing
+ * the run-time address of startup_32 [i.e. la(X)(%reg)], or as an
+ * immediate [$ la(X)].
+ *
+ * This macro can only be used from within the .head.text section, since the
+ * expression requires startup_32 to be in the same section as the code being
+ * assembled.
+ */
+#define la(X) ((X) - startup_32)
+
 	.code32
 SYM_FUNC_START(startup_32)
 	/*
@@ -64,10 +81,10 @@ SYM_FUNC_START(startup_32)
 	leal	(BP_scratch+4)(%esi), %esp
 	call	1f
 1:	popl	%ebp
-	subl	$1b, %ebp
+	subl	$ la(1b), %ebp
 
 	/* Load new GDT with the 64bit segments using 32bit descriptor */
-	leal	gdt(%ebp), %eax
+	leal	la(gdt)(%ebp), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
@@ -80,7 +97,7 @@ SYM_FUNC_START(startup_32)
 	movl	%eax, %ss
 
 /* setup a stack and make sure cpu supports long mode. */
-	leal	boot_stack_end(%ebp), %esp
+	leal	la(boot_stack_end)(%ebp), %esp
 
 	call	verify_cpu
 	testl	%eax, %eax
@@ -107,7 +124,7 @@ SYM_FUNC_START(startup_32)
  *	image_offset = startup_32 - image_base
  * Otherwise image_offset will be zero and has no effect on the calculations.
  */
-	subl    image_offset(%ebp), %ebx
+	subl    la(image_offset)(%ebp), %ebx
 #endif
 
 	movl	BP_kernel_alignment(%esi), %eax
@@ -123,7 +140,7 @@ SYM_FUNC_START(startup_32)
 
 	/* Target address to relocate to for decompression */
 	addl	BP_init_size(%esi), %ebx
-	subl	$_end, %ebx
+	subl	$ la(_end), %ebx
 
 /*
  * Prepare for entering 64 bit mode
@@ -151,19 +168,19 @@ SYM_FUNC_START(startup_32)
 1:
 
 	/* Initialize Page tables to 0 */
-	leal	pgtable(%ebx), %edi
+	leal	la(pgtable)(%ebx), %edi
 	xorl	%eax, %eax
 	movl	$(BOOT_INIT_PGT_SIZE/4), %ecx
 	rep	stosl
 
 	/* Build Level 4 */
-	leal	pgtable + 0(%ebx), %edi
+	leal	la(pgtable + 0)(%ebx), %edi
 	leal	0x1007 (%edi), %eax
 	movl	%eax, 0(%edi)
 	addl	%edx, 4(%edi)
 
 	/* Build Level 3 */
-	leal	pgtable + 0x1000(%ebx), %edi
+	leal	la(pgtable + 0x1000)(%ebx), %edi
 	leal	0x1007(%edi), %eax
 	movl	$4, %ecx
 1:	movl	%eax, 0x00(%edi)
@@ -174,7 +191,7 @@ SYM_FUNC_START(startup_32)
 	jnz	1b
 
 	/* Build Level 2 */
-	leal	pgtable + 0x2000(%ebx), %edi
+	leal	la(pgtable + 0x2000)(%ebx), %edi
 	movl	$0x00000183, %eax
 	movl	$2048, %ecx
 1:	movl	%eax, 0(%edi)
@@ -185,7 +202,7 @@ SYM_FUNC_START(startup_32)
 	jnz	1b
 
 	/* Enable the boot page tables */
-	leal	pgtable(%ebx), %eax
+	leal	la(pgtable)(%ebx), %eax
 	movl	%eax, %cr3
 
 	/* Enable Long mode in EFER (Extended Feature Enable Register) */
@@ -211,17 +228,17 @@ SYM_FUNC_START(startup_32)
 	 * used to perform that far jump.
 	 */
 	pushl	$__KERNEL_CS
-	leal	startup_64(%ebp), %eax
+	leal	la(startup_64)(%ebp), %eax
 #ifdef CONFIG_EFI_MIXED
-	movl	efi32_boot_args(%ebp), %edi
+	movl	la(efi32_boot_args)(%ebp), %edi
 	cmp	$0, %edi
 	jz	1f
-	leal	efi64_stub_entry(%ebp), %eax
-	movl	efi32_boot_args+4(%ebp), %esi
-	movl	efi32_boot_args+8(%ebp), %edx	// saved bootparams pointer
+	leal	la(efi64_stub_entry)(%ebp), %eax
+	movl	la(efi32_boot_args+4)(%ebp), %esi
+	movl	la(efi32_boot_args+8)(%ebp), %edx	// saved bootparams pointer
 	cmpl	$0, %edx
 	jnz	1f
-	leal	efi_pe_entry(%ebp), %eax
+	leal	la(efi_pe_entry)(%ebp), %eax
 	movl	%edi, %ecx			// MS calling convention
 	movl	%esi, %edx
 1:
@@ -246,18 +263,18 @@ SYM_FUNC_START(efi32_stub_entry)
 
 	call	1f
 1:	pop	%ebp
-	subl	$1b, %ebp
+	subl	$ la(1b), %ebp
 
-	movl	%esi, efi32_boot_args+8(%ebp)
+	movl	%esi, la(efi32_boot_args+8)(%ebp)
 SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
-	movl	%ecx, efi32_boot_args(%ebp)
-	movl	%edx, efi32_boot_args+4(%ebp)
-	movb	$0, efi_is64(%ebp)
+	movl	%ecx, la(efi32_boot_args)(%ebp)
+	movl	%edx, la(efi32_boot_args+4)(%ebp)
+	movb	$0, la(efi_is64)(%ebp)
 
 	/* Save firmware GDTR and code/data selectors */
-	sgdtl	efi32_boot_gdt(%ebp)
-	movw	%cs, efi32_boot_cs(%ebp)
-	movw	%ds, efi32_boot_ds(%ebp)
+	sgdtl	la(efi32_boot_gdt)(%ebp)
+	movw	%cs, la(efi32_boot_cs)(%ebp)
+	movw	%ds, la(efi32_boot_ds)(%ebp)
 
 	/* Disable paging */
 	movl	%cr0, %eax
@@ -336,11 +353,11 @@ SYM_CODE_START(startup_64)
 
 	/* Target address to relocate to for decompression */
 	movl	BP_init_size(%rsi), %ebx
-	subl	$_end, %ebx
+	subl	$ la(_end), %ebx
 	addq	%rbp, %rbx
 
 	/* Set up the stack */
-	leaq	boot_stack_end(%rbx), %rsp
+	leaq	la(boot_stack_end)(%rbx), %rsp
 
 	/*
 	 * At this point we are in long mode with 4-level paging enabled,
@@ -406,7 +423,7 @@ SYM_CODE_START(startup_64)
 	lretq
 trampoline_return:
 	/* Restore the stack, the 32-bit trampoline uses its own stack */
-	leaq	boot_stack_end(%rbx), %rsp
+	leaq	la(boot_stack_end)(%rbx), %rsp
 
 	/*
 	 * cleanup_trampoline() would restore trampoline memory.
@@ -418,7 +435,7 @@ trampoline_return:
 	 * this function call.
 	 */
 	pushq	%rsi
-	leaq	top_pgtable(%rbx), %rdi
+	leaq	la(top_pgtable)(%rbx), %rdi
 	call	cleanup_trampoline
 	popq	%rsi
 
@@ -432,9 +449,9 @@ trampoline_return:
  */
 	pushq	%rsi
 	leaq	(_bss-8)(%rip), %rsi
-	leaq	(_bss-8)(%rbx), %rdi
-	movq	$_bss /* - $startup_32 */, %rcx
-	shrq	$3, %rcx
+	leaq	la(_bss-8)(%rbx), %rdi
+	movl	$(_bss - startup_32), %ecx
+	shrl	$3, %ecx
 	std
 	rep	movsq
 	cld
@@ -445,15 +462,15 @@ trampoline_return:
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
 	 * to the new copy of the GDT.
 	 */
-	leaq	gdt64(%rbx), %rax
-	leaq	gdt(%rbx), %rdx
+	leaq	la(gdt64)(%rbx), %rax
+	leaq	la(gdt)(%rbx), %rdx
 	movq	%rdx, 2(%rax)
 	lgdt	(%rax)
 
 /*
  * Jump to the relocated address.
  */
-	leaq	.Lrelocated(%rbx), %rax
+	leaq	la(.Lrelocated)(%rbx), %rax
 	jmp	*%rax
 SYM_CODE_END(startup_64)
 
@@ -465,7 +482,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
 	movq	%rdx, %rbx			/* save boot_params pointer */
 	call	efi_main
 	movq	%rbx,%rsi
-	leaq	startup_64(%rax), %rax
+	leaq	la(startup_64)(%rax), %rax
 	jmp	*%rax
 SYM_FUNC_END(efi64_stub_entry)
 SYM_FUNC_END_ALIAS(efi_stub_entry)
@@ -628,7 +645,7 @@ SYM_DATA(efi_is64, .byte 1)
 #define BS32_handle_protocol	88 // offsetof(efi_boot_services_32_t, handle_protocol)
 #define LI32_image_base		32 // offsetof(efi_loaded_image_32_t, image_base)
 
-	.text
+	__HEAD
 	.code32
 SYM_FUNC_START(efi32_pe_entry)
 /*
@@ -650,12 +667,12 @@ SYM_FUNC_START(efi32_pe_entry)
 
 	call	1f
 1:	pop	%ebx
-	subl	$1b, %ebx
+	subl	$ la(1b), %ebx
 
 	/* Get the loaded image protocol pointer from the image handle */
 	leal	-4(%ebp), %eax
 	pushl	%eax				// &loaded_image
-	leal	loaded_image_proto(%ebx), %eax
+	leal	la(loaded_image_proto)(%ebx), %eax
 	pushl	%eax				// pass the GUID address
 	pushl	8(%ebp)				// pass the image handle
 
@@ -690,7 +707,7 @@ SYM_FUNC_START(efi32_pe_entry)
 	 * use it before we get to the 64-bit efi_pe_entry() in C code.
 	 */
 	subl	%esi, %ebx
-	movl	%ebx, image_offset(%ebp)	// save image_offset
+	movl	%ebx, la(image_offset)(%ebp)	// save image_offset
 	jmp	efi32_pe_stub_entry
 
 2:	popl	%edi				// restore callee-save registers
-- 
2.26.2

[PATCH 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S

[Arvind Sankar]

The BFD linker generates runtime relocations for z_input_len and
z_output_len, even though they are absolute symbols.

Work around this by defining two variables input_len and output_len in
addition to the symbols, and use them via position-independent
references.

This eliminates the last two runtime relocations in the head code and
allows us to drop the -z noreloc-overflow flag to the linker.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/Makefile  |  8 --------
 arch/x86/boot/compressed/head_32.S | 17 ++++++++---------
 arch/x86/boot/compressed/head_64.S |  4 ++--
 arch/x86/boot/compressed/mkpiggy.c |  6 ++++++
 4 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index aa9ed814e5fa..d3e882e855ee 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -49,15 +49,7 @@ UBSAN_SANITIZE :=n
 KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
 # Compressed kernel should be built as PIE since it may be loaded at any
 # address by the bootloader.
-ifeq ($(CONFIG_X86_32),y)
 KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
-else
-# To build 64-bit compressed kernel as PIE, we disable relocation
-# overflow check to avoid relocation overflow error with a new linker
-# command-line option, -z noreloc-overflow.
-KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
-	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
-endif
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 66657bb99aae..064e895bad92 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -193,18 +193,17 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 /*
  * Do the extraction, and jump to the new kernel..
  */
-				/* push arguments for extract_kernel: */
-	pushl	$z_output_len	/* decompressed length, end of relocs */
+	/* push arguments for extract_kernel: */
 
-	pushl	%ebp		/* output address */
-
-	pushl	$z_input_len	/* input_len */
+	pushl	output_len@GOTOFF(%ebx)	/* decompressed length, end of relocs */
+	pushl	%ebp			/* output address */
+	pushl	input_len@GOTOFF(%ebx)	/* input_len */
 	leal	input_data@GOTOFF(%ebx), %eax
-	pushl	%eax		/* input_data */
+	pushl	%eax			/* input_data */
 	leal	boot_heap@GOTOFF(%ebx), %eax
-	pushl	%eax		/* heap area */
-	pushl	%esi		/* real mode pointer */
-	call	extract_kernel	/* returns kernel location in %eax */
+	pushl	%eax			/* heap area */
+	pushl	%esi			/* real mode pointer */
+	call	extract_kernel		/* returns kernel location in %eax */
 	addl	$24, %esp
 
 /*
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index f6ba32cd5702..6e4704b6a94e 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -508,9 +508,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	movq	%rsi, %rdi		/* real mode address */
 	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
 	leaq	input_data(%rip), %rdx  /* input_data */
-	movl	$z_input_len, %ecx	/* input_len */
+	movl	input_len(%rip), %ecx	/* input_len */
 	movq	%rbp, %r8		/* output target address */
-	movl	$z_output_len, %r9d	/* decompressed length, end of relocs */
+	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
 	call	extract_kernel		/* returns kernel location in %rax */
 	popq	%rsi
 
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index 7e01248765b2..52aa56cdbacc 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -60,6 +60,12 @@ int main(int argc, char *argv[])
 	printf(".incbin \"%s\"\n", argv[1]);
 	printf("input_data_end:\n");
 
+	printf(".section \".rodata\",\"a\",@progbits\n");
+	printf(".globl input_len\n");
+	printf("input_len:\n\t.long %lu\n", ilen);
+	printf(".globl output_len\n");
+	printf("output_len:\n\t.long %lu\n", (unsigned long)olen);
+
 	retval = 0;
 bail:
 	if (f)
-- 
2.26.2

[PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

Add a linker script check that there are no runtime relocations, and
remove the old one that tries to check via looking for specially-named
sections in the object files.

Drop the tests for -fPIE compiler option and -pie linker option, as they
are available in all supported gcc and binutils versions (as well as
clang and lld).

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
 arch/x86/boot/compressed/vmlinux.lds.S | 11 ++++++++++
 2 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index d3e882e855ee..679a2b383bfe 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
 	vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
 
 KBUILD_CFLAGS := -m$(BITS) -O2
-KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
+KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
 cflags-$(CONFIG_X86_32) := -march=i386
 cflags-$(CONFIG_X86_64) := -mcmodel=small
@@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
 KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
 # Compressed kernel should be built as PIE since it may be loaded at any
 # address by the bootloader.
-KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
+KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy
@@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
 
-# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
-# can place it anywhere in memory and it will still run. However, since
-# it is executed as-is without any ELF relocation processing performed
-# (and has already had all relocation sections stripped from the binary),
-# none of the code can use data relocations (e.g. static assignments of
-# pointer values), since they will be meaningless at runtime. This check
-# will refuse to link the vmlinux if any of these relocations are found.
-quiet_cmd_check_data_rel = DATAREL $@
-define cmd_check_data_rel
-	for obj in $(filter %.o,$^); do \
-		$(READELF) -S $$obj | grep -qF .rel.local && { \
-			echo "error: $$obj has data relocations!" >&2; \
-			exit 1; \
-		} || true; \
-	done
-endef
-
-# We need to run two commands under "if_changed", so merge them into a
-# single invocation.
-quiet_cmd_check-and-link-vmlinux = LD      $@
-      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
-
 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
-	$(call if_changed,check-and-link-vmlinux)
+	$(call if_changed,ld)
 
 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index d826ab38a8f9..0ac14feacb24 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -11,9 +11,15 @@ OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
 #ifdef CONFIG_X86_64
 OUTPUT_ARCH(i386:x86-64)
 ENTRY(startup_64)
+
+#define REL .rela
+
 #else
 OUTPUT_ARCH(i386)
 ENTRY(startup_32)
+
+#define REL .rel
+
 #endif
 
 SECTIONS
@@ -42,6 +48,9 @@ SECTIONS
 		*(.rodata.*)
 		_erodata = . ;
 	}
+	REL.dyn : {
+		*(REL.*)
+	}
 	.got : {
 		*(.got)
 	}
@@ -83,3 +92,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
 #else
 ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
 #endif
+
+ASSERT(SIZEOF(REL.dyn) == 0, "Unexpected runtime relocations detected!")
-- 
2.26.2

Re: [PATCH 1/4] x86/boot: Add .text.startup to setup.ld

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>gcc puts the main function into .text.startup when compiled with -Os (or
>-O2). This results in arch/x86/boot/main.c having a .text.startup
>section which is currently not included explicitly in the linker script
>setup.ld in the same directory.
>
>The BFD linker places this orphan section immediately after .text, so
>this still works. However, LLD git, since [1], is choosing to place it
>immediately after the .bstext section instead (this is the first code
>section). This plays havoc with the section layout that setup.elf
>requires to create the setup header, for eg on 64-bit:
>
>    LD      arch/x86/boot/setup.elf
>  ld.lld: error: section .text.startup file range overlaps with .header
>  >>> .text.startup range is [0x200040, 0x2001FE]
>  >>> .header range is [0x2001EF, 0x20026B]
>
>  ld.lld: error: section .header file range overlaps with .bsdata
>  >>> .header range is [0x2001EF, 0x20026B]
>  >>> .bsdata range is [0x2001FF, 0x200398]
>
>  ld.lld: error: section .bsdata file range overlaps with .entrytext
>  >>> .bsdata range is [0x2001FF, 0x200398]
>  >>> .entrytext range is [0x20026C, 0x2002D3]
>
>  ld.lld: error: section .text.startup virtual address range overlaps
>  with .header
>  >>> .text.startup range is [0x40, 0x1FE]
>  >>> .header range is [0x1EF, 0x26B]
>
>  ld.lld: error: section .header virtual address range overlaps with
>  .bsdata
>  >>> .header range is [0x1EF, 0x26B]
>  >>> .bsdata range is [0x1FF, 0x398]
>
>  ld.lld: error: section .bsdata virtual address range overlaps with
>  .entrytext
>  >>> .bsdata range is [0x1FF, 0x398]
>  >>> .entrytext range is [0x26C, 0x2D3]
>
>  ld.lld: error: section .text.startup load address range overlaps with
>  .header
>  >>> .text.startup range is [0x40, 0x1FE]
>  >>> .header range is [0x1EF, 0x26B]
>
>  ld.lld: error: section .header load address range overlaps with
>  .bsdata
>  >>> .header range is [0x1EF, 0x26B]
>  >>> .bsdata range is [0x1FF, 0x398]
>
>  ld.lld: error: section .bsdata load address range overlaps with
>  .entrytext
>  >>> .bsdata range is [0x1FF, 0x398]
>  >>> .entrytext range is [0x26C, 0x2D3]
>
>Explicitly pull .text.startup into the .text output section to avoid
>this.
>
>[1] https://reviews.llvm.org/D75225
>
>Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>Reviewed-by: Fangrui Song <maskray@google.com>
>---
> arch/x86/boot/setup.ld | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
>index 24c95522f231..ed60abcdb089 100644
>--- a/arch/x86/boot/setup.ld
>+++ b/arch/x86/boot/setup.ld
>@@ -20,7 +20,7 @@ SECTIONS
> 	.initdata	: { *(.initdata) }
> 	__end_init = .;
>
>-	.text		: { *(.text) }
>+	.text		: { *(.text.startup) *(.text) }
> 	.text32		: { *(.text32) }
>
> 	. = ALIGN(16);
>-- 
>2.26.2

Should .text.startup* be used instead? If -ffunction-sections is used,

// a.c
int main() {}

gcc -O2 a.c                     # .text.startup
gcc -Os a.c                     # .text.startup

gcc -O2 -ffunction-sections a.c # .text.startup.main
gcc -Os -ffunction-sections a.c # .text.startup.main

-----

In case anyone wants to CC a GCC dev for the citation that 
  main compiles to `.text.startup` in -Os or -O2 mode, I have a small request
  that `.text.startup.` probably makes more sense. See

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95095

I made an llvm change recently https://reviews.llvm.org/D79600

Re: [PATCH 1/4] x86/boot: Add .text.startup to setup.ld

[Brian Gerst]

On Sun, May 24, 2020 at 5:32 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> gcc puts the main function into .text.startup when compiled with -Os (or
> -O2). This results in arch/x86/boot/main.c having a .text.startup
> section which is currently not included explicitly in the linker script
> setup.ld in the same directory.

If the compiler is making assumptions based on the function name
"main" wouldn't it be simpler just to rename the function?

--
Brian Gerst

Re: [PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>The assembly code in head_{32,64}.S, while meant to be
>position-independent, generates run-time relocations because it uses
>instructions such as
>	leal	gdt(%edx), %eax
>which make the assembler and linker think that the code is using %edx as
>an index into gdt, and hence gdt needs to be relocated to its run-time
>address.
>
>With the BFD linker, this generates a warning during the build:
>  LD      arch/x86/boot/compressed/vmlinux
>ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
>ld: warning: creating a DT_TEXTREL in object

Interesting. How does the build generate a warning by default?
Do you use Gentoo Linux which appears to ship a --warn-shared-textrel
enabled-by-default patch? (https://bugs.gentoo.org/700488)

% cat a.s
leal    gdt(%edx), %eax
% as --32 a.s -o a.o
% ld.bfd -m elf_i386 -shared a.o -z notext # DT_TEXTREL is set. R_386_32

% ld.bfd -m elf_i386 -shared a.o           # on-demand text relocations. DT_TEXTREL is set. R_386_32

% ld.bfd -m elf_i386 -shared a.o --warn-shared-textrel
ld.bfd: a.o: warning: relocation against `gdt' in read-only section `.text'
ld.bfd: warning: creating a DT_TEXTREL in a shared object

% ld.bfd -m elf_i386 -shared a.o -z text
ld.bfd: a.o: warning: relocation against `gdt' in read-only section `.text'
ld.bfd: read-only segment has dynamic relocations
## The above is an error. Output is suppressed

lld has only two modes: -z text (default) and -z notext. There is no
on-demand state. By default it will error.

There are feature requests to make -z text default, at least for some
architectures. I just found
https://sourceware.org/bugzilla/show_bug.cgi?id=20824

>With lld, Dmitry Golovin reports that this results in a link-time error
>with default options (i.e. unless -z notext is explicitly passed):
>  LD      arch/x86/boot/compressed/vmlinux
>ld.lld: error: can't create dynamic relocation R_386_32 against local
>symbol in readonly segment; recompile object files with -fPIC or pass
>'-Wl,-z,notext' to allow text relocations in the output
>
>Start fixing this by removing relocations from .head.text:
>- On 32-bit, use a base register that holds the address of the GOT and
>  reference symbol addresses using @GOTOFF, i.e.
>	leal	gdt@GOTOFF(%edx), %eax

Looks good to me.

>- On 64-bit, most of the code can (and already does) use %rip-relative
>  addressing, however the .code32 bits can't, and the 64-bit code also
>  needs to reference symbol addresses as they will be after moving the
>  compressed kernel to the end of the decompression buffer.
>  For these cases, reference the symbols as an offset to startup_32 to
>  avoid creating relocations, i.e.
>  	leal	(gdt-startup_32)(%bp), %eax
>  This only works in .head.text as the subtraction cannot be represented
>  as a PC-relative relocation unless startup_32 is in the same section
>  as the code. Move efi32_pe_entry into .head.text so that it can use
>  the same method to avoid relocations.

I have a nit about the startup_32 comment. See below.

>Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>---
> arch/x86/boot/compressed/head_32.S | 40 +++++++------
> arch/x86/boot/compressed/head_64.S | 95 ++++++++++++++++++------------
> 2 files changed, 77 insertions(+), 58 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
>index dfa4131c65df..66657bb99aae 100644
>--- a/arch/x86/boot/compressed/head_32.S
>+++ b/arch/x86/boot/compressed/head_32.S
>@@ -73,10 +73,10 @@ SYM_FUNC_START(startup_32)
> 	leal	(BP_scratch+4)(%esi), %esp
> 	call	1f
> 1:	popl	%edx
>-	subl	$1b, %edx
>+	addl	$_GLOBAL_OFFSET_TABLE_+(.-1b), %edx
>
> 	/* Load new GDT */
>-	leal	gdt(%edx), %eax
>+	leal	gdt@GOTOFF(%edx), %eax
> 	movl	%eax, 2(%eax)
> 	lgdt	(%eax)
>
>@@ -89,14 +89,16 @@ SYM_FUNC_START(startup_32)
> 	movl	%eax, %ss
>
> /*
>- * %edx contains the address we are loaded at by the boot loader and %ebx
>- * contains the address where we should move the kernel image temporarily
>- * for safe in-place decompression. %ebp contains the address that the kernel
>- * will be decompressed to.
>+ * %edx contains the address we are loaded at by the boot loader (plus the
>+ * offset to the GOT).  The below code calculates %ebx to be the address where
>+ * we should move the kernel image temporarily for safe in-place decompression
>+ * (again, plus the offset to the GOT).
>+ *
>+ * %ebp is calculated to be the address that the kernel will be decompressed to.
>  */
>
> #ifdef CONFIG_RELOCATABLE
>-	movl	%edx, %ebx
>+	leal	startup_32@GOTOFF(%edx), %ebx
>
> #ifdef CONFIG_EFI_STUB
> /*
>@@ -107,7 +109,7 @@ SYM_FUNC_START(startup_32)
>  *	image_offset = startup_32 - image_base
>  * Otherwise image_offset will be zero and has no effect on the calculations.
>  */
>-	subl    image_offset(%edx), %ebx
>+	subl    image_offset@GOTOFF(%edx), %ebx
> #endif
>
> 	movl	BP_kernel_alignment(%esi), %eax
>@@ -124,10 +126,10 @@ SYM_FUNC_START(startup_32)
> 	movl	%ebx, %ebp	// Save the output address for later
> 	/* Target address to relocate to for decompression */
> 	addl    BP_init_size(%esi), %ebx
>-	subl    $_end, %ebx
>+	subl    $_end@GOTOFF, %ebx
>
> 	/* Set up the stack */
>-	leal	boot_stack_end(%ebx), %esp
>+	leal	boot_stack_end@GOTOFF(%ebx), %esp
>
> 	/* Zero EFLAGS */
> 	pushl	$0
>@@ -138,8 +140,8 @@ SYM_FUNC_START(startup_32)
>  * where decompression in place becomes safe.
>  */
> 	pushl	%esi
>-	leal	(_bss-4)(%edx), %esi
>-	leal	(_bss-4)(%ebx), %edi
>+	leal	(_bss@GOTOFF-4)(%edx), %esi
>+	leal	(_bss@GOTOFF-4)(%ebx), %edi
> 	movl	$(_bss - startup_32), %ecx
> 	shrl	$2, %ecx
> 	std
>@@ -152,14 +154,14 @@ SYM_FUNC_START(startup_32)
> 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
> 	 * to the new copy of the GDT.
> 	 */
>-	leal	gdt(%ebx), %eax
>+	leal	gdt@GOTOFF(%ebx), %eax
> 	movl	%eax, 2(%eax)
> 	lgdt	(%eax)
>
> /*
>  * Jump to the relocated address.
>  */
>-	leal	.Lrelocated(%ebx), %eax
>+	leal	.Lrelocated@GOTOFF(%ebx), %eax
> 	jmp	*%eax
> SYM_FUNC_END(startup_32)
>
>@@ -169,7 +171,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
> 	add	$0x4, %esp
> 	movl	8(%esp), %esi	/* save boot_params pointer */
> 	call	efi_main
>-	leal	startup_32(%eax), %eax
>+	/* efi_main returns the possibly relocated address of startup_32 */
> 	jmp	*%eax
> SYM_FUNC_END(efi32_stub_entry)
> SYM_FUNC_END_ALIAS(efi_stub_entry)
>@@ -182,8 +184,8 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
>  * Clear BSS (stack is currently empty)
>  */
> 	xorl	%eax, %eax
>-	leal	_bss(%ebx), %edi
>-	leal	_ebss(%ebx), %ecx
>+	leal	_bss@GOTOFF(%ebx), %edi
>+	leal	_ebss@GOTOFF(%ebx), %ecx
> 	subl	%edi, %ecx
> 	shrl	$2, %ecx
> 	rep	stosl
>@@ -197,9 +199,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> 	pushl	%ebp		/* output address */
>
> 	pushl	$z_input_len	/* input_len */
>-	leal	input_data(%ebx), %eax
>+	leal	input_data@GOTOFF(%ebx), %eax
> 	pushl	%eax		/* input_data */
>-	leal	boot_heap(%ebx), %eax
>+	leal	boot_heap@GOTOFF(%ebx), %eax
> 	pushl	%eax		/* heap area */
> 	pushl	%esi		/* real mode pointer */
> 	call	extract_kernel	/* returns kernel location in %eax */
>diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
>index 706fbf6eef53..f6ba32cd5702 100644
>--- a/arch/x86/boot/compressed/head_64.S
>+++ b/arch/x86/boot/compressed/head_64.S
>@@ -42,6 +42,23 @@
> 	.hidden _ebss
>
> 	__HEAD
>+
>+/*
>+ * This macro gives the link address of X. It's the same as X, since startup_32
>+ * has link address 0, but defining it this way tells the assembler/linker that
>+ * we want the link address, and not the run-time address of X. This prevents
>+ * the linker from creating a run-time relocation entry for this reference.
>+ * The macro should be used as a displacement with a base register containing
>+ * the run-time address of startup_32 [i.e. la(X)(%reg)], or as an
>+ * immediate [$ la(X)].
>+ *
>+ * This macro can only be used from within the .head.text section, since the
>+ * expression requires startup_32 to be in the same section as the code being
>+ * assembled.
>+ */
>+#define la(X) ((X) - startup_32)
>+

IIRC, %ebp contains the address of startup_32. la(X) references X
relative to startup_32. The fixup (in GNU as and clang integrated
assembler's term) is a constant which is resolved by the assembler.

There is no R_386_32 or R_386_PC32 for the linker to resolve.

Not very sure stating that "since startup_32 has link address 0" is very
appropriate here (probably because I did't see the term "link address"
before). If my understanding above is correct, I think you can just
reword the comment to express that X is referenced relative to
startup_32, which is stored in %ebp.

> 	.code32
> SYM_FUNC_START(startup_32)
> 	/*
>@@ -64,10 +81,10 @@ SYM_FUNC_START(startup_32)
> 	leal	(BP_scratch+4)(%esi), %esp
> 	call	1f
> 1:	popl	%ebp
>-	subl	$1b, %ebp
>+	subl	$ la(1b), %ebp
>
> 	/* Load new GDT with the 64bit segments using 32bit descriptor */
>-	leal	gdt(%ebp), %eax
>+	leal	la(gdt)(%ebp), %eax
> 	movl	%eax, 2(%eax)
> 	lgdt	(%eax)
>
>@@ -80,7 +97,7 @@ SYM_FUNC_START(startup_32)
> 	movl	%eax, %ss
>
> /* setup a stack and make sure cpu supports long mode. */
>-	leal	boot_stack_end(%ebp), %esp
>+	leal	la(boot_stack_end)(%ebp), %esp
>
> 	call	verify_cpu
> 	testl	%eax, %eax
>@@ -107,7 +124,7 @@ SYM_FUNC_START(startup_32)
>  *	image_offset = startup_32 - image_base
>  * Otherwise image_offset will be zero and has no effect on the calculations.
>  */
>-	subl    image_offset(%ebp), %ebx
>+	subl    la(image_offset)(%ebp), %ebx
> #endif
>
> 	movl	BP_kernel_alignment(%esi), %eax
>@@ -123,7 +140,7 @@ SYM_FUNC_START(startup_32)
>
> 	/* Target address to relocate to for decompression */
> 	addl	BP_init_size(%esi), %ebx
>-	subl	$_end, %ebx
>+	subl	$ la(_end), %ebx
>
> /*
>  * Prepare for entering 64 bit mode
>@@ -151,19 +168,19 @@ SYM_FUNC_START(startup_32)
> 1:
>
> 	/* Initialize Page tables to 0 */
>-	leal	pgtable(%ebx), %edi
>+	leal	la(pgtable)(%ebx), %edi
> 	xorl	%eax, %eax
> 	movl	$(BOOT_INIT_PGT_SIZE/4), %ecx
> 	rep	stosl
>
> 	/* Build Level 4 */
>-	leal	pgtable + 0(%ebx), %edi
>+	leal	la(pgtable + 0)(%ebx), %edi
> 	leal	0x1007 (%edi), %eax
> 	movl	%eax, 0(%edi)
> 	addl	%edx, 4(%edi)
>
> 	/* Build Level 3 */
>-	leal	pgtable + 0x1000(%ebx), %edi
>+	leal	la(pgtable + 0x1000)(%ebx), %edi
> 	leal	0x1007(%edi), %eax
> 	movl	$4, %ecx
> 1:	movl	%eax, 0x00(%edi)
>@@ -174,7 +191,7 @@ SYM_FUNC_START(startup_32)
> 	jnz	1b
>
> 	/* Build Level 2 */
>-	leal	pgtable + 0x2000(%ebx), %edi
>+	leal	la(pgtable + 0x2000)(%ebx), %edi
> 	movl	$0x00000183, %eax
> 	movl	$2048, %ecx
> 1:	movl	%eax, 0(%edi)
>@@ -185,7 +202,7 @@ SYM_FUNC_START(startup_32)
> 	jnz	1b
>
> 	/* Enable the boot page tables */
>-	leal	pgtable(%ebx), %eax
>+	leal	la(pgtable)(%ebx), %eax
> 	movl	%eax, %cr3
>
> 	/* Enable Long mode in EFER (Extended Feature Enable Register) */
>@@ -211,17 +228,17 @@ SYM_FUNC_START(startup_32)
> 	 * used to perform that far jump.
> 	 */
> 	pushl	$__KERNEL_CS
>-	leal	startup_64(%ebp), %eax
>+	leal	la(startup_64)(%ebp), %eax
> #ifdef CONFIG_EFI_MIXED
>-	movl	efi32_boot_args(%ebp), %edi
>+	movl	la(efi32_boot_args)(%ebp), %edi
> 	cmp	$0, %edi
> 	jz	1f
>-	leal	efi64_stub_entry(%ebp), %eax
>-	movl	efi32_boot_args+4(%ebp), %esi
>-	movl	efi32_boot_args+8(%ebp), %edx	// saved bootparams pointer
>+	leal	la(efi64_stub_entry)(%ebp), %eax
>+	movl	la(efi32_boot_args+4)(%ebp), %esi
>+	movl	la(efi32_boot_args+8)(%ebp), %edx	// saved bootparams pointer
> 	cmpl	$0, %edx
> 	jnz	1f
>-	leal	efi_pe_entry(%ebp), %eax
>+	leal	la(efi_pe_entry)(%ebp), %eax
> 	movl	%edi, %ecx			// MS calling convention
> 	movl	%esi, %edx
> 1:
>@@ -246,18 +263,18 @@ SYM_FUNC_START(efi32_stub_entry)
>
> 	call	1f
> 1:	pop	%ebp
>-	subl	$1b, %ebp
>+	subl	$ la(1b), %ebp
>
>-	movl	%esi, efi32_boot_args+8(%ebp)
>+	movl	%esi, la(efi32_boot_args+8)(%ebp)
> SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
>-	movl	%ecx, efi32_boot_args(%ebp)
>-	movl	%edx, efi32_boot_args+4(%ebp)
>-	movb	$0, efi_is64(%ebp)
>+	movl	%ecx, la(efi32_boot_args)(%ebp)
>+	movl	%edx, la(efi32_boot_args+4)(%ebp)
>+	movb	$0, la(efi_is64)(%ebp)
>
> 	/* Save firmware GDTR and code/data selectors */
>-	sgdtl	efi32_boot_gdt(%ebp)
>-	movw	%cs, efi32_boot_cs(%ebp)
>-	movw	%ds, efi32_boot_ds(%ebp)
>+	sgdtl	la(efi32_boot_gdt)(%ebp)
>+	movw	%cs, la(efi32_boot_cs)(%ebp)
>+	movw	%ds, la(efi32_boot_ds)(%ebp)
>
> 	/* Disable paging */
> 	movl	%cr0, %eax
>@@ -336,11 +353,11 @@ SYM_CODE_START(startup_64)
>
> 	/* Target address to relocate to for decompression */
> 	movl	BP_init_size(%rsi), %ebx
>-	subl	$_end, %ebx
>+	subl	$ la(_end), %ebx
> 	addq	%rbp, %rbx
>
> 	/* Set up the stack */
>-	leaq	boot_stack_end(%rbx), %rsp
>+	leaq	la(boot_stack_end)(%rbx), %rsp
>
> 	/*
> 	 * At this point we are in long mode with 4-level paging enabled,
>@@ -406,7 +423,7 @@ SYM_CODE_START(startup_64)
> 	lretq
> trampoline_return:
> 	/* Restore the stack, the 32-bit trampoline uses its own stack */
>-	leaq	boot_stack_end(%rbx), %rsp
>+	leaq	la(boot_stack_end)(%rbx), %rsp
>
> 	/*
> 	 * cleanup_trampoline() would restore trampoline memory.
>@@ -418,7 +435,7 @@ trampoline_return:
> 	 * this function call.
> 	 */
> 	pushq	%rsi
>-	leaq	top_pgtable(%rbx), %rdi
>+	leaq	la(top_pgtable)(%rbx), %rdi
> 	call	cleanup_trampoline
> 	popq	%rsi
>
>@@ -432,9 +449,9 @@ trampoline_return:
>  */
> 	pushq	%rsi
> 	leaq	(_bss-8)(%rip), %rsi
>-	leaq	(_bss-8)(%rbx), %rdi
>-	movq	$_bss /* - $startup_32 */, %rcx
>-	shrq	$3, %rcx
>+	leaq	la(_bss-8)(%rbx), %rdi
>+	movl	$(_bss - startup_32), %ecx
>+	shrl	$3, %ecx
> 	std
> 	rep	movsq
> 	cld
>@@ -445,15 +462,15 @@ trampoline_return:
> 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
> 	 * to the new copy of the GDT.
> 	 */
>-	leaq	gdt64(%rbx), %rax
>-	leaq	gdt(%rbx), %rdx
>+	leaq	la(gdt64)(%rbx), %rax
>+	leaq	la(gdt)(%rbx), %rdx
> 	movq	%rdx, 2(%rax)
> 	lgdt	(%rax)
>
> /*
>  * Jump to the relocated address.
>  */
>-	leaq	.Lrelocated(%rbx), %rax
>+	leaq	la(.Lrelocated)(%rbx), %rax
> 	jmp	*%rax
> SYM_CODE_END(startup_64)
>
>@@ -465,7 +482,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
> 	movq	%rdx, %rbx			/* save boot_params pointer */
> 	call	efi_main
> 	movq	%rbx,%rsi
>-	leaq	startup_64(%rax), %rax
>+	leaq	la(startup_64)(%rax), %rax
> 	jmp	*%rax
> SYM_FUNC_END(efi64_stub_entry)
> SYM_FUNC_END_ALIAS(efi_stub_entry)
>@@ -628,7 +645,7 @@ SYM_DATA(efi_is64, .byte 1)
> #define BS32_handle_protocol	88 // offsetof(efi_boot_services_32_t, handle_protocol)
> #define LI32_image_base		32 // offsetof(efi_loaded_image_32_t, image_base)
>
>-	.text
>+	__HEAD
> 	.code32
> SYM_FUNC_START(efi32_pe_entry)
> /*
>@@ -650,12 +667,12 @@ SYM_FUNC_START(efi32_pe_entry)
>
> 	call	1f
> 1:	pop	%ebx
>-	subl	$1b, %ebx
>+	subl	$ la(1b), %ebx
>
> 	/* Get the loaded image protocol pointer from the image handle */
> 	leal	-4(%ebp), %eax
> 	pushl	%eax				// &loaded_image
>-	leal	loaded_image_proto(%ebx), %eax
>+	leal	la(loaded_image_proto)(%ebx), %eax
> 	pushl	%eax				// pass the GUID address
> 	pushl	8(%ebp)				// pass the image handle
>
>@@ -690,7 +707,7 @@ SYM_FUNC_START(efi32_pe_entry)
> 	 * use it before we get to the 64-bit efi_pe_entry() in C code.
> 	 */
> 	subl	%esi, %ebx
>-	movl	%ebx, image_offset(%ebp)	// save image_offset
>+	movl	%ebx, la(image_offset)(%ebp)	// save image_offset
> 	jmp	efi32_pe_stub_entry
>
> 2:	popl	%edi				// restore callee-save registers
>-- 
>2.26.2
>

Re: [PATCH 1/4] x86/boot: Add .text.startup to setup.ld

[Arvind Sankar]

On Sun, May 24, 2020 at 03:13:26PM -0700, Fangrui Song wrote:
> On 2020-05-24, Arvind Sankar wrote:
> >gcc puts the main function into .text.startup when compiled with -Os (or
> >-O2). This results in arch/x86/boot/main.c having a .text.startup
> >section which is currently not included explicitly in the linker script
> >setup.ld in the same directory.
> >
> >The BFD linker places this orphan section immediately after .text, so
> >this still works. However, LLD git, since [1], is choosing to place it
> >immediately after the .bstext section instead (this is the first code
> >section). This plays havoc with the section layout that setup.elf
> >requires to create the setup header, for eg on 64-bit:
> >
> >    LD      arch/x86/boot/setup.elf
> >  ld.lld: error: section .text.startup file range overlaps with .header
> >  >>> .text.startup range is [0x200040, 0x2001FE]
> >  >>> .header range is [0x2001EF, 0x20026B]
> >
> >  ld.lld: error: section .header file range overlaps with .bsdata
> >  >>> .header range is [0x2001EF, 0x20026B]
> >  >>> .bsdata range is [0x2001FF, 0x200398]
> >
> >  ld.lld: error: section .bsdata file range overlaps with .entrytext
> >  >>> .bsdata range is [0x2001FF, 0x200398]
> >  >>> .entrytext range is [0x20026C, 0x2002D3]
> >
> >  ld.lld: error: section .text.startup virtual address range overlaps
> >  with .header
> >  >>> .text.startup range is [0x40, 0x1FE]
> >  >>> .header range is [0x1EF, 0x26B]
> >
> >  ld.lld: error: section .header virtual address range overlaps with
> >  .bsdata
> >  >>> .header range is [0x1EF, 0x26B]
> >  >>> .bsdata range is [0x1FF, 0x398]
> >
> >  ld.lld: error: section .bsdata virtual address range overlaps with
> >  .entrytext
> >  >>> .bsdata range is [0x1FF, 0x398]
> >  >>> .entrytext range is [0x26C, 0x2D3]
> >
> >  ld.lld: error: section .text.startup load address range overlaps with
> >  .header
> >  >>> .text.startup range is [0x40, 0x1FE]
> >  >>> .header range is [0x1EF, 0x26B]
> >
> >  ld.lld: error: section .header load address range overlaps with
> >  .bsdata
> >  >>> .header range is [0x1EF, 0x26B]
> >  >>> .bsdata range is [0x1FF, 0x398]
> >
> >  ld.lld: error: section .bsdata load address range overlaps with
> >  .entrytext
> >  >>> .bsdata range is [0x1FF, 0x398]
> >  >>> .entrytext range is [0x26C, 0x2D3]
> >
> >Explicitly pull .text.startup into the .text output section to avoid
> >this.
> >
> >[1] https://reviews.llvm.org/D75225
> >
> >Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> >Reviewed-by: Fangrui Song <maskray@google.com>
> >---
> > arch/x86/boot/setup.ld | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
> >index 24c95522f231..ed60abcdb089 100644
> >--- a/arch/x86/boot/setup.ld
> >+++ b/arch/x86/boot/setup.ld
> >@@ -20,7 +20,7 @@ SECTIONS
> > 	.initdata	: { *(.initdata) }
> > 	__end_init = .;
> >
> >-	.text		: { *(.text) }
> >+	.text		: { *(.text.startup) *(.text) }
> > 	.text32		: { *(.text32) }
> >
> > 	. = ALIGN(16);
> >-- 
> >2.26.2
> 
> Should .text.startup* be used instead? If -ffunction-sections is used,
> 
> // a.c
> int main() {}
> 
> gcc -O2 a.c                     # .text.startup
> gcc -Os a.c                     # .text.startup
> 
> gcc -O2 -ffunction-sections a.c # .text.startup.main
> gcc -Os -ffunction-sections a.c # .text.startup.main

It's probably unlikely we'll use function-sections on the setup code,
but *(.text.*) might be more future-proof, since gcc/clang might grow
the ability to stick code into .text.hot or .text.unlikely etc
automatically.

> 
> -----
> 
> In case anyone wants to CC a GCC dev for the citation that 
>   main compiles to `.text.startup` in -Os or -O2 mode, I have a small request
>   that `.text.startup.` probably makes more sense. See
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95095
> 
> I made an llvm change recently https://reviews.llvm.org/D79600

Re: [PATCH 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>The BFD linker generates runtime relocations for z_input_len and
>z_output_len, even though they are absolute symbols.
>
>Work around this by defining two variables input_len and output_len in
>addition to the symbols, and use them via position-independent
>references.
>
>This eliminates the last two runtime relocations in the head code and
>allows us to drop the -z noreloc-overflow flag to the linker.
>
>Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>---
> arch/x86/boot/compressed/Makefile  |  8 --------
> arch/x86/boot/compressed/head_32.S | 17 ++++++++---------
> arch/x86/boot/compressed/head_64.S |  4 ++--
> arch/x86/boot/compressed/mkpiggy.c |  6 ++++++
> 4 files changed, 16 insertions(+), 19 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>index aa9ed814e5fa..d3e882e855ee 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -49,15 +49,7 @@ UBSAN_SANITIZE :=n
> KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
> # Compressed kernel should be built as PIE since it may be loaded at any
> # address by the bootloader.
>-ifeq ($(CONFIG_X86_32),y)
> KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
>-else
>-# To build 64-bit compressed kernel as PIE, we disable relocation
>-# overflow check to avoid relocation overflow error with a new linker
>-# command-line option, -z noreloc-overflow.
>-KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
>-	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
>-endif
> LDFLAGS_vmlinux := -T
>
> hostprogs	:= mkpiggy
>diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
>index 66657bb99aae..064e895bad92 100644
>--- a/arch/x86/boot/compressed/head_32.S
>+++ b/arch/x86/boot/compressed/head_32.S
>@@ -193,18 +193,17 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> /*
>  * Do the extraction, and jump to the new kernel..
>  */
>-				/* push arguments for extract_kernel: */
>-	pushl	$z_output_len	/* decompressed length, end of relocs */
>+	/* push arguments for extract_kernel: */
>
>-	pushl	%ebp		/* output address */
>-
>-	pushl	$z_input_len	/* input_len */
>+	pushl	output_len@GOTOFF(%ebx)	/* decompressed length, end of relocs */
>+	pushl	%ebp			/* output address */
>+	pushl	input_len@GOTOFF(%ebx)	/* input_len */
> 	leal	input_data@GOTOFF(%ebx), %eax
>-	pushl	%eax		/* input_data */
>+	pushl	%eax			/* input_data */
> 	leal	boot_heap@GOTOFF(%ebx), %eax
>-	pushl	%eax		/* heap area */
>-	pushl	%esi		/* real mode pointer */
>-	call	extract_kernel	/* returns kernel location in %eax */
>+	pushl	%eax			/* heap area */
>+	pushl	%esi			/* real mode pointer */
>+	call	extract_kernel		/* returns kernel location in %eax */
> 	addl	$24, %esp
>
> /*
>diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
>index f6ba32cd5702..6e4704b6a94e 100644
>--- a/arch/x86/boot/compressed/head_64.S
>+++ b/arch/x86/boot/compressed/head_64.S
>@@ -508,9 +508,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> 	movq	%rsi, %rdi		/* real mode address */
> 	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
> 	leaq	input_data(%rip), %rdx  /* input_data */
>-	movl	$z_input_len, %ecx	/* input_len */
>+	movl	input_len(%rip), %ecx	/* input_len */
> 	movq	%rbp, %r8		/* output target address */
>-	movl	$z_output_len, %r9d	/* decompressed length, end of relocs */
>+	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
> 	call	extract_kernel		/* returns kernel location in %rax */
> 	popq	%rsi
>
>diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
>index 7e01248765b2..52aa56cdbacc 100644
>--- a/arch/x86/boot/compressed/mkpiggy.c
>+++ b/arch/x86/boot/compressed/mkpiggy.c
>@@ -60,6 +60,12 @@ int main(int argc, char *argv[])
> 	printf(".incbin \"%s\"\n", argv[1]);
> 	printf("input_data_end:\n");
>
>+	printf(".section \".rodata\",\"a\",@progbits\n");
>+	printf(".globl input_len\n");
>+	printf("input_len:\n\t.long %lu\n", ilen);
>+	printf(".globl output_len\n");
>+	printf("output_len:\n\t.long %lu\n", (unsigned long)olen);
>+
> 	retval = 0;
> bail:
> 	if (f)
>-- 
>2.26.2
>

Probably worth mentioning that this works around GNU ld<2.35


This commit fixing https://sourceware.org/bugzilla/show_bug.cgi?id=25754
also fixed the bug. (Just verified that both 2.24 and 2.34 have the bug. binutils-gdb HEAD (future 2.35) is good.)

% cat a.s
pushl $z_input_len
% cat b.s
.globl z_input_len
z_input_len = 0xb612
% gcc -m32 -c a.s b.s
% ld.bfd -m elf_i386 -pie a.o b.o  # has an incorrect R_386_RELATIVE before binutils 2.35


Reviewed-by: Fangrui Song <maskray@google.com>

Re: [PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>Add a linker script check that there are no runtime relocations, and
>remove the old one that tries to check via looking for specially-named
>sections in the object files.
>
>Drop the tests for -fPIE compiler option and -pie linker option, as they
>are available in all supported gcc and binutils versions (as well as
>clang and lld).
>
>Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>---
> arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
> arch/x86/boot/compressed/vmlinux.lds.S | 11 ++++++++++
> 2 files changed, 14 insertions(+), 25 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>index d3e882e855ee..679a2b383bfe 100644
>--- a/arch/x86/boot/compressed/Makefile
>+++ b/arch/x86/boot/compressed/Makefile
>@@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
> 	vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
>
> KBUILD_CFLAGS := -m$(BITS) -O2
>-KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
>+KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
> KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
> cflags-$(CONFIG_X86_32) := -march=i386
> cflags-$(CONFIG_X86_64) := -mcmodel=small
>@@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
> KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
> # Compressed kernel should be built as PIE since it may be loaded at any
> # address by the bootloader.
>-KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
>+KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
> LDFLAGS_vmlinux := -T
>
> hostprogs	:= mkpiggy
>@@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
> vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>
>-# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
>-# can place it anywhere in memory and it will still run. However, since
>-# it is executed as-is without any ELF relocation processing performed
>-# (and has already had all relocation sections stripped from the binary),
>-# none of the code can use data relocations (e.g. static assignments of
>-# pointer values), since they will be meaningless at runtime. This check
>-# will refuse to link the vmlinux if any of these relocations are found.
>-quiet_cmd_check_data_rel = DATAREL $@
>-define cmd_check_data_rel
>-	for obj in $(filter %.o,$^); do \
>-		$(READELF) -S $$obj | grep -qF .rel.local && { \
>-			echo "error: $$obj has data relocations!" >&2; \
>-			exit 1; \
>-		} || true; \
>-	done
>-endef
>-
>-# We need to run two commands under "if_changed", so merge them into a
>-# single invocation.
>-quiet_cmd_check-and-link-vmlinux = LD      $@
>-      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>-
> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>-	$(call if_changed,check-and-link-vmlinux)
>+	$(call if_changed,ld)
>
> OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
> $(obj)/vmlinux.bin: vmlinux FORCE
>diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>index d826ab38a8f9..0ac14feacb24 100644
>--- a/arch/x86/boot/compressed/vmlinux.lds.S
>+++ b/arch/x86/boot/compressed/vmlinux.lds.S
>@@ -11,9 +11,15 @@ OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
> #ifdef CONFIG_X86_64
> OUTPUT_ARCH(i386:x86-64)
> ENTRY(startup_64)
>+
>+#define REL .rela
>+
> #else
> OUTPUT_ARCH(i386)
> ENTRY(startup_32)
>+
>+#define REL .rel
>+
> #endif
>
> SECTIONS
>@@ -42,6 +48,9 @@ SECTIONS
> 		*(.rodata.*)
> 		_erodata = . ;
> 	}
>+	REL.dyn : {
>+		*(REL.*)
>+	}
> 	.got : {
> 		*(.got)
> 	}
>@@ -83,3 +92,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
> #else
> ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
> #endif
>+
>+ASSERT(SIZEOF(REL.dyn) == 0, "Unexpected runtime relocations detected!")
>-- 
>2.26.2
>

`grep -qF .rel.local` from 98f78525371b55ccd1c480207ce10296c72fa340
may be incorrect.. None of these synthesized dynamic relocation sections is
called *.rel.local* ...
(it probably wanted to name .rel.data.rel.ro or .rel.data)


Reviewed-by: Fangrui Song <maskray@google.com>

Re: [PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code

[Arvind Sankar]

On Sun, May 24, 2020 at 03:53:59PM -0700, Fangrui Song wrote:
> On 2020-05-24, Arvind Sankar wrote:
> >The assembly code in head_{32,64}.S, while meant to be
> >position-independent, generates run-time relocations because it uses
> >instructions such as
> >	leal	gdt(%edx), %eax
> >which make the assembler and linker think that the code is using %edx as
> >an index into gdt, and hence gdt needs to be relocated to its run-time
> >address.
> >
> >With the BFD linker, this generates a warning during the build:
> >  LD      arch/x86/boot/compressed/vmlinux
> >ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
> >ld: warning: creating a DT_TEXTREL in object
> 
> Interesting. How does the build generate a warning by default?
> Do you use Gentoo Linux which appears to ship a --warn-shared-textrel
> enabled-by-default patch? (https://bugs.gentoo.org/700488)

Ah, yes I am using gentoo. I didn't realize that was a distro
modification.

> >+
> >+/*
> >+ * This macro gives the link address of X. It's the same as X, since startup_32
> >+ * has link address 0, but defining it this way tells the assembler/linker that
> >+ * we want the link address, and not the run-time address of X. This prevents
> >+ * the linker from creating a run-time relocation entry for this reference.
> >+ * The macro should be used as a displacement with a base register containing
> >+ * the run-time address of startup_32 [i.e. la(X)(%reg)], or as an
> >+ * immediate [$ la(X)].
> >+ *
> >+ * This macro can only be used from within the .head.text section, since the
> >+ * expression requires startup_32 to be in the same section as the code being
> >+ * assembled.
> >+ */
> >+#define la(X) ((X) - startup_32)
> >+
> 
> IIRC, %ebp contains the address of startup_32. la(X) references X
> relative to startup_32. The fixup (in GNU as and clang integrated
> assembler's term) is a constant which is resolved by the assembler.
> 
> There is no R_386_32 or R_386_PC32 for the linker to resolve.

This is incorrect (or maybe I'm not understanding you correctly). X is a
symbol whose final location relative to startup_32 is in most cases not
known to the assembler (there are a couple of cases where X is a label
within .head.text which do get completely resolved by the assembler).

For example, taking the instruction loading the gdt address, this is
what we get from the assembler:
	lea	la(gdt)(%ebp), %eax
becomes in the object file:
  11:   8d 85 00 00 00 00       lea    0x0(%ebp),%eax
			13: R_X86_64_PC32       .data+0x23
or a cleaner example using a global symbol:
	subl	la(image_offset)(%ebp), %ebx
becomes
  41:   2b 9d 00 00 00 00       sub    0x0(%ebp),%ebx
			43: R_X86_64_PC32       image_offset+0x43

So in general you get PC32 relocations, with the addend being set by the
assembler to .-startup_32, modulo the adjustment for where within the
instruction the displacement needs to be. These relocations are resolved
by the static linker to produce constants in the final executable.


> 
> Not very sure stating that "since startup_32 has link address 0" is very
> appropriate here (probably because I did't see the term "link address"
> before). If my understanding above is correct, I think you can just
> reword the comment to express that X is referenced relative to
> startup_32, which is stored in %ebp.
> 

Yeah, the more standard term is virtual address/VMA, but that sounds
confusing to me with PIE code since the _actual_ virtual address at
which this code is going to run isn't 0, that's just the address assumed
for linking. I can reword it to avoid referencing "link address" but
then it's not obvious why the macro is named "la" :) I'm open to
suggestions on a better name, I could use offset but that's a bit
long-winded. I could just use vma() if nobody else finds it confusing.

Thanks.

Re: [PATCH 1/4] x86/boot: Add .text.startup to setup.ld

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>On Sun, May 24, 2020 at 03:13:26PM -0700, Fangrui Song wrote:
>> On 2020-05-24, Arvind Sankar wrote:
>> >gcc puts the main function into .text.startup when compiled with -Os (or
>> >-O2). This results in arch/x86/boot/main.c having a .text.startup
>> >section which is currently not included explicitly in the linker script
>> >setup.ld in the same directory.
>> >
>> >The BFD linker places this orphan section immediately after .text, so
>> >this still works. However, LLD git, since [1], is choosing to place it
>> >immediately after the .bstext section instead (this is the first code
>> >section). This plays havoc with the section layout that setup.elf
>> >requires to create the setup header, for eg on 64-bit:
>> >
>> >    LD      arch/x86/boot/setup.elf
>> >  ld.lld: error: section .text.startup file range overlaps with .header
>> >  >>> .text.startup range is [0x200040, 0x2001FE]
>> >  >>> .header range is [0x2001EF, 0x20026B]
>> >
>> >  ld.lld: error: section .header file range overlaps with .bsdata
>> >  >>> .header range is [0x2001EF, 0x20026B]
>> >  >>> .bsdata range is [0x2001FF, 0x200398]
>> >
>> >  ld.lld: error: section .bsdata file range overlaps with .entrytext
>> >  >>> .bsdata range is [0x2001FF, 0x200398]
>> >  >>> .entrytext range is [0x20026C, 0x2002D3]
>> >
>> >  ld.lld: error: section .text.startup virtual address range overlaps
>> >  with .header
>> >  >>> .text.startup range is [0x40, 0x1FE]
>> >  >>> .header range is [0x1EF, 0x26B]
>> >
>> >  ld.lld: error: section .header virtual address range overlaps with
>> >  .bsdata
>> >  >>> .header range is [0x1EF, 0x26B]
>> >  >>> .bsdata range is [0x1FF, 0x398]
>> >
>> >  ld.lld: error: section .bsdata virtual address range overlaps with
>> >  .entrytext
>> >  >>> .bsdata range is [0x1FF, 0x398]
>> >  >>> .entrytext range is [0x26C, 0x2D3]
>> >
>> >  ld.lld: error: section .text.startup load address range overlaps with
>> >  .header
>> >  >>> .text.startup range is [0x40, 0x1FE]
>> >  >>> .header range is [0x1EF, 0x26B]
>> >
>> >  ld.lld: error: section .header load address range overlaps with
>> >  .bsdata
>> >  >>> .header range is [0x1EF, 0x26B]
>> >  >>> .bsdata range is [0x1FF, 0x398]
>> >
>> >  ld.lld: error: section .bsdata load address range overlaps with
>> >  .entrytext
>> >  >>> .bsdata range is [0x1FF, 0x398]
>> >  >>> .entrytext range is [0x26C, 0x2D3]
>> >
>> >Explicitly pull .text.startup into the .text output section to avoid
>> >this.
>> >
>> >[1] https://reviews.llvm.org/D75225
>> >
>> >Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>> >Reviewed-by: Fangrui Song <maskray@google.com>
>> >---
>> > arch/x86/boot/setup.ld | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> >diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
>> >index 24c95522f231..ed60abcdb089 100644
>> >--- a/arch/x86/boot/setup.ld
>> >+++ b/arch/x86/boot/setup.ld
>> >@@ -20,7 +20,7 @@ SECTIONS
>> > 	.initdata	: { *(.initdata) }
>> > 	__end_init = .;
>> >
>> >-	.text		: { *(.text) }
>> >+	.text		: { *(.text.startup) *(.text) }
>> > 	.text32		: { *(.text32) }
>> >
>> > 	. = ALIGN(16);
>> >--
>> >2.26.2
>>
>> Should .text.startup* be used instead? If -ffunction-sections is used,
>>
>> // a.c
>> int main() {}
>>
>> gcc -O2 a.c                     # .text.startup
>> gcc -Os a.c                     # .text.startup
>>
>> gcc -O2 -ffunction-sections a.c # .text.startup.main
>> gcc -Os -ffunction-sections a.c # .text.startup.main
>
>It's probably unlikely we'll use function-sections on the setup code,
>but *(.text.*) might be more future-proof, since gcc/clang might grow
>the ability to stick code into .text.hot or .text.unlikely etc
>automatically.

*(.text.*) looks good to me. When you send PATCH v2, feel free to add

(There is indeed no guarantee that future clang FDO will not place it .text.unknown. :)

Reviewed-by: Fangrui Song <maskray@google.com>

>>
>> -----
>>
>> In case anyone wants to CC a GCC dev for the citation that
>>   main compiles to `.text.startup` in -Os or -O2 mode, I have a small request
>>   that `.text.startup.` probably makes more sense. See
>>
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95095
>>
>> I made an llvm change recently https://reviews.llvm.org/D79600

Re: [PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

On Sun, May 24, 2020 at 04:36:07PM -0700, Fangrui Song wrote:
> 
> `grep -qF .rel.local` from 98f78525371b55ccd1c480207ce10296c72fa340
> may be incorrect.. None of these synthesized dynamic relocation sections is
> called *.rel.local* ...
> (it probably wanted to name .rel.data.rel.ro or .rel.data)
> 
> 
> Reviewed-by: Fangrui Song <maskray@google.com>

At least from gcc you get .data.rel.local sections if you have, for eg:
	struct { void *p; } foo = { .p = &bar };
where bar is defined in the same file. These aren't relocation sections,
foo is actually placed in the .data.rel.local section instead of .data.

But yeah, it's incomplete, you wouldn't catch it if bar was external
(foo goes in .data.rel) or foo was const (foo goes in .data.rel.ro*).

Re: [PATCH 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S

[Arvind Sankar]

On Sun, May 24, 2020 at 04:22:14PM -0700, Fangrui Song wrote:
> 
> Probably worth mentioning that this works around GNU ld<2.35

Thanks, I'll add that in v2.

> 
> 
> This commit fixing https://sourceware.org/bugzilla/show_bug.cgi?id=25754
> also fixed the bug. (Just verified that both 2.24 and 2.34 have the bug. binutils-gdb HEAD (future 2.35) is good.)
> 
> % cat a.s
> pushl $z_input_len
> % cat b.s
> .globl z_input_len
> z_input_len = 0xb612
> % gcc -m32 -c a.s b.s
> % ld.bfd -m elf_i386 -pie a.o b.o  # has an incorrect R_386_RELATIVE before binutils 2.35
> 
> 
> Reviewed-by: Fangrui Song <maskray@google.com>

Re: [PATCH 2/4] x86/boot: Remove runtime relocations from .head.text code

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>On Sun, May 24, 2020 at 03:53:59PM -0700, Fangrui Song wrote:
>> On 2020-05-24, Arvind Sankar wrote:
>> >The assembly code in head_{32,64}.S, while meant to be
>> >position-independent, generates run-time relocations because it uses
>> >instructions such as
>> >	leal	gdt(%edx), %eax
>> >which make the assembler and linker think that the code is using %edx as
>> >an index into gdt, and hence gdt needs to be relocated to its run-time
>> >address.
>> >
>> >With the BFD linker, this generates a warning during the build:
>> >  LD      arch/x86/boot/compressed/vmlinux
>> >ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
>> >ld: warning: creating a DT_TEXTREL in object
>>
>> Interesting. How does the build generate a warning by default?
>> Do you use Gentoo Linux which appears to ship a --warn-shared-textrel
>> enabled-by-default patch? (https://bugs.gentoo.org/700488)
>
>Ah, yes I am using gentoo. I didn't realize that was a distro
>modification.
>
>> >+
>> >+/*
>> >+ * This macro gives the link address of X. It's the same as X, since startup_32
>> >+ * has link address 0, but defining it this way tells the assembler/linker that
>> >+ * we want the link address, and not the run-time address of X. This prevents
>> >+ * the linker from creating a run-time relocation entry for this reference.
>> >+ * The macro should be used as a displacement with a base register containing
>> >+ * the run-time address of startup_32 [i.e. la(X)(%reg)], or as an
>> >+ * immediate [$ la(X)].
>> >+ *
>> >+ * This macro can only be used from within the .head.text section, since the
>> >+ * expression requires startup_32 to be in the same section as the code being
>> >+ * assembled.
>> >+ */
>> >+#define la(X) ((X) - startup_32)
>> >+
>>
>> IIRC, %ebp contains the address of startup_32. la(X) references X
>> relative to startup_32. The fixup (in GNU as and clang integrated
>> assembler's term) is a constant which is resolved by the assembler.
>>
>> There is no R_386_32 or R_386_PC32 for the linker to resolve.
>
>This is incorrect (or maybe I'm not understanding you correctly). X is a
>symbol whose final location relative to startup_32 is in most cases not
>known to the assembler (there are a couple of cases where X is a label
>within .head.text which do get completely resolved by the assembler).
>
>For example, taking the instruction loading the gdt address, this is
>what we get from the assembler:
>	lea	la(gdt)(%ebp), %eax
>becomes in the object file:
>  11:   8d 85 00 00 00 00       lea    0x0(%ebp),%eax
>			13: R_X86_64_PC32       .data+0x23
>or a cleaner example using a global symbol:
>	subl	la(image_offset)(%ebp), %ebx
>becomes
>  41:   2b 9d 00 00 00 00       sub    0x0(%ebp),%ebx
>			43: R_X86_64_PC32       image_offset+0x43
>
>So in general you get PC32 relocations, with the addend being set by the
>assembler to .-startup_32, modulo the adjustment for where within the
>instruction the displacement needs to be. These relocations are resolved
>by the static linker to produce constants in the final executable.
>

You are right. I am not familiar with the code and only looked at 1b.

Just preprocessed head_64.S and verified many target symbols are in
.data and .pgtable  The assembler converts an expression `foo - symbol_defined_in_same_section`
to be `foo - . + offset` which can be encoded as an R_X86_64_PC32 (or
resolved the fixup if it is a constant, e.g. `1b - startup_32`)

>>
>> Not very sure stating that "since startup_32 has link address 0" is very
>> appropriate here (probably because I did't see the term "link address"
>> before). If my understanding above is correct, I think you can just
>> reword the comment to express that X is referenced relative to
>> startup_32, which is stored in %ebp.
>>
>
>Yeah, the more standard term is virtual address/VMA, but that sounds
>confusing to me with PIE code since the _actual_ virtual address at
>which this code is going to run isn't 0, that's just the address assumed
>for linking. I can reword it to avoid referencing "link address" but
>then it's not obvious why the macro is named "la" :) I'm open to
>suggestions on a better name, I could use offset but that's a bit
>long-winded. I could just use vma() if nobody else finds it confusing.
>
>Thanks.

With your approach, the important property is that "the distance between
startup_32 and the target symbol is a constant", not that "startup_32
has link address 0".  `ra`, `rva`, `rvma` or any other term which expresses "relative"
should work.  Hope someone can come up with a good suggestion:)

Re: [PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Ard Biesheuvel]

On Sun, 24 May 2020 at 23:28, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> Add a linker script check that there are no runtime relocations, and
> remove the old one that tries to check via looking for specially-named
> sections in the object files.
>
> Drop the tests for -fPIE compiler option and -pie linker option, as they
> are available in all supported gcc and binutils versions (as well as
> clang and lld).
>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
>  arch/x86/boot/compressed/vmlinux.lds.S | 11 ++++++++++
>  2 files changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index d3e882e855ee..679a2b383bfe 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
>         vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
>
>  KBUILD_CFLAGS := -m$(BITS) -O2
> -KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
> +KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
>  KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
>  cflags-$(CONFIG_X86_32) := -march=i386
>  cflags-$(CONFIG_X86_64) := -mcmodel=small
> @@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
>  KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
>  # Compressed kernel should be built as PIE since it may be loaded at any
>  # address by the bootloader.
> -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
>  LDFLAGS_vmlinux := -T
>
>  hostprogs      := mkpiggy
> @@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>
> -# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
> -# can place it anywhere in memory and it will still run. However, since
> -# it is executed as-is without any ELF relocation processing performed
> -# (and has already had all relocation sections stripped from the binary),
> -# none of the code can use data relocations (e.g. static assignments of
> -# pointer values), since they will be meaningless at runtime. This check
> -# will refuse to link the vmlinux if any of these relocations are found.
> -quiet_cmd_check_data_rel = DATAREL $@
> -define cmd_check_data_rel
> -       for obj in $(filter %.o,$^); do \
> -               $(READELF) -S $$obj | grep -qF .rel.local && { \
> -                       echo "error: $$obj has data relocations!" >&2; \
> -                       exit 1; \
> -               } || true; \
> -       done
> -endef
> -
> -# We need to run two commands under "if_changed", so merge them into a
> -# single invocation.
> -quiet_cmd_check-and-link-vmlinux = LD      $@
> -      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
> -
>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> -       $(call if_changed,check-and-link-vmlinux)
> +       $(call if_changed,ld)
>
>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>  $(obj)/vmlinux.bin: vmlinux FORCE
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index d826ab38a8f9..0ac14feacb24 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -11,9 +11,15 @@ OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
>  #ifdef CONFIG_X86_64
>  OUTPUT_ARCH(i386:x86-64)
>  ENTRY(startup_64)
> +
> +#define REL .rela
> +
>  #else
>  OUTPUT_ARCH(i386)
>  ENTRY(startup_32)
> +
> +#define REL .rel
> +
>  #endif
>
>  SECTIONS
> @@ -42,6 +48,9 @@ SECTIONS
>                 *(.rodata.*)
>                 _erodata = . ;
>         }
> +       REL.dyn : {
> +               *(REL.*)
> +       }

Do we really need the macro here? Could we just do

.rel.dyn : { *(.rel.* .rela.*) }

(or even

.rel.dyn  : { *(.rel.* }
.rela.dyn : { *(.rela.*) }

if the output section name matters, and always assert that both are empty)?

>         .got : {
>                 *(.got)
>         }
> @@ -83,3 +92,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
>  #else
>  ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
>  #endif
> +
> +ASSERT(SIZEOF(REL.dyn) == 0, "Unexpected runtime relocations detected!")
> --
> 2.26.2
>

Re: [PATCH 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Ard Biesheuvel]

On Sun, 24 May 2020 at 23:28, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> The compressed kernel currently contains bogus runtime relocations in
> the startup code in head_{32,64}.S, which are generated by the linker,
> but must not actually be processed at runtime.
>
> This generates warnings when linking with the BFD linker, and errors
> with LLD, which defaults to erroring on runtime relocations in read-only
> sections. It also requires the -z noreloc-overflow hack for the 64-bit
> kernel, which prevents us from linking it as -pie on an older BFD linker
> (<= 2.26) or on LLD, because the locations that are to be apparently
> relocated are only 32-bits in size and so cannot normally have
> R_X86_64_RELATIVE relocations.
>
> This series aims to get rid of these relocations. It is based on
> efi/next (efi-changes-for-v5.8), where the latest patches touch the
> head code to eliminate the global offset table.
>

Note: I dropped my decompressor linker script changes from that tag,
but they are still at the top of the efi/next branch.

Given these changes to go on top, I think it is better to merge all of
them separately, and let the x86 maintainers decide how and when.
(I can prepare a branch and a separate PR if desired)

For the series (modulo one nit in a separate reply)

Reviewed-by: Ard Biesheuvel <ardb@kernel.org>



> The first patch is an independent fix for LLD, to avoid an orphan
> section in arch/x86/boot/setup.elf [0].
>
> The second patch gets rid of almost all the relocations. It uses
> standard PIC addressing technique for 32-bit, i.e. loading a register
> with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> references to access variables. For 64-bit, there is 32-bit code that
> cannot use RIP-relative addressing, and also cannot use the 32-bit
> method, since GOTOFF references are 64-bit only. This is instead handled
> using a macro to replace a reference like gdt with (gdt-startup_32)
> instead. The assembler will generate a PC32 relocation entry, with
> addend set to (.-startup_32), and these will be replaced with constants
> at link time. This works as long as all the code using such references
> lives in the same section as startup_32, i.e. in .head.text.
>
> The third patch addresses a remaining issue with the BFD linker, which
> insists on generating runtime relocations for absolute symbols. We use
> z_input_len and z_output_len, defined in the generated piggy.S file, as
> symbols whose absolute "addresses" are actually the size of the
> compressed payload and the size of the decompressed kernel image
> respectively. LLD does not generate relocations for these two symbols,
> but the BFD linker does. To get around this, piggy.S is extended to also
> define two u32 variables (in .rodata) with the lengths, and the head
> code is modified to use those instead of the symbol addresses.
>
> An alternative way to handle z_input_len/z_output_len would be to just
> include piggy.S in head_{32,64}.S instead of as a separate object file,
> since the GNU assembler doesn't generate relocations for symbols set to
> constants.
>
> The last patch adds a check in the linker script to ensure that no
> runtime relocations get reintroduced. Since the GOT has been eliminated
> as well, the compressed kernel has no runtime relocations whatsoever any
> more.
>
> [0] https://lore.kernel.org/lkml/20200521152459.558081-1-nivedita@alum.mit.edu/
>
> Arvind Sankar (4):
>   x86/boot: Add .text.startup to setup.ld
>   x86/boot: Remove runtime relocations from .head.text code
>   x86/boot: Remove runtime relocations from head_{32,64}.S
>   x86/boot: Check that there are no runtime relocations
>
>  arch/x86/boot/compressed/Makefile      | 36 +---------
>  arch/x86/boot/compressed/head_32.S     | 59 +++++++--------
>  arch/x86/boot/compressed/head_64.S     | 99 +++++++++++++++-----------
>  arch/x86/boot/compressed/mkpiggy.c     |  6 ++
>  arch/x86/boot/compressed/vmlinux.lds.S | 11 +++
>  arch/x86/boot/setup.ld                 |  2 +-
>  6 files changed, 109 insertions(+), 104 deletions(-)
>
> --
> 2.26.2
>

Re: [PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Fangrui Song]

On 2020-05-25, Ard Biesheuvel wrote:
>On Sun, 24 May 2020 at 23:28, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>>
>> Add a linker script check that there are no runtime relocations, and
>> remove the old one that tries to check via looking for specially-named
>> sections in the object files.
>>
>> Drop the tests for -fPIE compiler option and -pie linker option, as they
>> are available in all supported gcc and binutils versions (as well as
>> clang and lld).
>>
>> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
>> ---
>>  arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
>>  arch/x86/boot/compressed/vmlinux.lds.S | 11 ++++++++++
>>  2 files changed, 14 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
>> index d3e882e855ee..679a2b383bfe 100644
>> --- a/arch/x86/boot/compressed/Makefile
>> +++ b/arch/x86/boot/compressed/Makefile
>> @@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
>>         vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
>>
>>  KBUILD_CFLAGS := -m$(BITS) -O2
>> -KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
>> +KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
>>  KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
>>  cflags-$(CONFIG_X86_32) := -march=i386
>>  cflags-$(CONFIG_X86_64) := -mcmodel=small
>> @@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
>>  KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
>>  # Compressed kernel should be built as PIE since it may be loaded at any
>>  # address by the bootloader.
>> -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
>> +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
>>  LDFLAGS_vmlinux := -T
>>
>>  hostprogs      := mkpiggy
>> @@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
>>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
>>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>>
>> -# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
>> -# can place it anywhere in memory and it will still run. However, since
>> -# it is executed as-is without any ELF relocation processing performed
>> -# (and has already had all relocation sections stripped from the binary),
>> -# none of the code can use data relocations (e.g. static assignments of
>> -# pointer values), since they will be meaningless at runtime. This check
>> -# will refuse to link the vmlinux if any of these relocations are found.
>> -quiet_cmd_check_data_rel = DATAREL $@
>> -define cmd_check_data_rel
>> -       for obj in $(filter %.o,$^); do \
>> -               $(READELF) -S $$obj | grep -qF .rel.local && { \
>> -                       echo "error: $$obj has data relocations!" >&2; \
>> -                       exit 1; \
>> -               } || true; \
>> -       done
>> -endef
>> -
>> -# We need to run two commands under "if_changed", so merge them into a
>> -# single invocation.
>> -quiet_cmd_check-and-link-vmlinux = LD      $@
>> -      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
>> -
>>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
>> -       $(call if_changed,check-and-link-vmlinux)
>> +       $(call if_changed,ld)
>>
>>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>>  $(obj)/vmlinux.bin: vmlinux FORCE
>> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>> index d826ab38a8f9..0ac14feacb24 100644
>> --- a/arch/x86/boot/compressed/vmlinux.lds.S
>> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
>> @@ -11,9 +11,15 @@ OUTPUT_FORMAT(CONFIG_OUTPUT_FORMAT)
>>  #ifdef CONFIG_X86_64
>>  OUTPUT_ARCH(i386:x86-64)
>>  ENTRY(startup_64)
>> +
>> +#define REL .rela
>> +
>>  #else
>>  OUTPUT_ARCH(i386)
>>  ENTRY(startup_32)
>> +
>> +#define REL .rel
>> +
>>  #endif
>>
>>  SECTIONS
>> @@ -42,6 +48,9 @@ SECTIONS
>>                 *(.rodata.*)
>>                 _erodata = . ;
>>         }
>> +       REL.dyn : {
>> +               *(REL.*)
>> +       }
>
>Do we really need the macro here? Could we just do

The output section name does not matter: it will be discarded by the linker.

>.rel.dyn : { *(.rel.* .rela.*) }

If for some reasons there is at least one SHT_REL and at least one
SHT_RELA, LLD will error "section type mismatch for .rel.dyn", while the
intended diagnostic is the assert below.

>(or even
>
>.rel.dyn  : { *(.rel.* }
>.rela.dyn : { *(.rela.*) }
>
>if the output section name matters, and always assert that both are empty)?

   .rel.dyn  : { *(.rel.* }
   .rela.dyn : { *(.rela.*) }

looks good to me.


FWIW I intend to add -z rel and -z rela to LLD: https://reviews.llvm.org/D80496#inline-738804
(binutils thread https://sourceware.org/pipermail/binutils/2020-May/111244.html)

In case someone builds the x86-64 kernel with -z rel, your suggested
input section description will work out of the box...


>>         .got : {
>>                 *(.got)
>>         }
>> @@ -83,3 +92,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
>>  #else
>>  ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
>>  #endif
>> +
>> +ASSERT(SIZEOF(REL.dyn) == 0, "Unexpected runtime relocations detected!")
>> --
>> 2.26.2
>>

Re: [PATCH 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

On Mon, May 25, 2020 at 09:26:26AM -0700, Fangrui Song wrote:
> On 2020-05-25, Ard Biesheuvel wrote:
> >
> >Do we really need the macro here? Could we just do
> 
> The output section name does not matter: it will be discarded by the linker.
> 
> >.rel.dyn : { *(.rel.* .rela.*) }
> 
> If for some reasons there is at least one SHT_REL and at least one
> SHT_RELA, LLD will error "section type mismatch for .rel.dyn", while the
> intended diagnostic is the assert below.
> 
> >(or even
> >
> >.rel.dyn  : { *(.rel.* }
> >.rela.dyn : { *(.rela.*) }
> >
> >if the output section name matters, and always assert that both are empty)?
> 
>    .rel.dyn  : { *(.rel.* }
>    .rela.dyn : { *(.rela.*) }
> 
> looks good to me.
> 
> 
> FWIW I intend to add -z rel and -z rela to LLD: https://reviews.llvm.org/D80496#inline-738804
> (binutils thread https://sourceware.org/pipermail/binutils/2020-May/111244.html)
> 
> In case someone builds the x86-64 kernel with -z rel, your suggested
> input section description will work out of the box...
> 

Ok with me.

[PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Arvind Sankar]

The compressed kernel currently contains bogus runtime relocations in
the startup code in head_{32,64}.S, which are generated by the linker,
but must not actually be processed at runtime.

This generates warnings when linking with the BFD linker, and errors
with LLD, which defaults to erroring on runtime relocations in read-only
sections. It also requires the -z noreloc-overflow hack for the 64-bit
kernel, which prevents us from linking it as -pie on an older BFD linker
(<= 2.26) or on LLD, because the locations that are to be apparently
relocated are only 32-bits in size and so cannot normally have
R_X86_64_RELATIVE relocations.

This series aims to get rid of these relocations. It is based on
efi/next, where the latest patches touch the head code to eliminate the
global offset table.

The first patch is an independent fix for LLD, to avoid an orphan
section in arch/x86/boot/setup.elf.

The second patch gets rid of almost all the relocations. It uses
standard PIC addressing technique for 32-bit, i.e. loading a register
with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
references to access variables. For 64-bit, there is 32-bit code that
cannot use RIP-relative addressing, and also cannot use the 32-bit
method, since GOTOFF references are 64-bit only. This is instead handled
using a macro to replace a reference like gdt with (gdt-startup_32)
instead. The assembler will generate a PC32 relocation entry, with
addend set to (.-startup_32), and these will be replaced with constants
at link time. This works as long as all the code using such references
lives in the same section as startup_32, i.e. in .head.text.

The third patch addresses a remaining issue with the BFD linker, which
insists on generating runtime relocations for absolute symbols. We use
z_input_len and z_output_len, defined in the generated piggy.S file, as
symbols whose absolute "addresses" are actually the size of the
compressed payload and the size of the decompressed kernel image
respectively. LLD does not generate relocations for these two symbols,
but the BFD linker does, prior to the upcoming 2.35. To get around this,
piggy.S is extended to also define two u32 variables (in .rodata) with
the lengths, and the head code is modified to use those instead of the
symbol addresses.

An alternative way to handle z_input_len/z_output_len would be to just
include piggy.S in head_{32,64}.S instead of as a separate object file,
since the GNU assembler doesn't generate relocations for symbols set to
constants.

The last patch adds a check in the linker script to ensure that no
runtime relocations get reintroduced. Since the GOT has been eliminated
as well, the compressed kernel has no runtime relocations whatsoever any
more.

Changes from v1:
- Add .text.* to setup.ld instead of just .text.startup
- Rename the la() macro introduced in the second patch for 64-bit to
  rva(), and rework the explanatory comment.
- In the last patch, check both .rel.dyn and .rela.dyn, instead of just
  one per arch.

Arvind Sankar (4):
  x86/boot: Add .text.* to setup.ld
  x86/boot: Remove run-time relocations from .head.text code
  x86/boot: Remove runtime relocations from head_{32,64}.S
  x86/boot: Check that there are no runtime relocations

 arch/x86/boot/compressed/Makefile      |  36 +--------
 arch/x86/boot/compressed/head_32.S     |  59 +++++++-------
 arch/x86/boot/compressed/head_64.S     | 108 +++++++++++++++----------
 arch/x86/boot/compressed/mkpiggy.c     |   6 ++
 arch/x86/boot/compressed/vmlinux.lds.S |   8 ++
 arch/x86/boot/setup.ld                 |   2 +-
 6 files changed, 115 insertions(+), 104 deletions(-)

-- 
2.26.2

[PATCH v2 1/4] x86/boot: Add .text.* to setup.ld

[Arvind Sankar]

gcc puts the main function into .text.startup when compiled with -Os (or
-O2). This results in arch/x86/boot/main.c having a .text.startup
section which is currently not included explicitly in the linker script
setup.ld in the same directory.

The BFD linker places this orphan section immediately after .text, so
this still works. However, LLD git, since [1], is choosing to place it
immediately after the .bstext section instead (this is the first code
section). This plays havoc with the section layout that setup.elf
requires to create the setup header, for eg on 64-bit:

    LD      arch/x86/boot/setup.elf
  ld.lld: error: section .text.startup file range overlaps with .header
  >>> .text.startup range is [0x200040, 0x2001FE]
  >>> .header range is [0x2001EF, 0x20026B]

  ld.lld: error: section .header file range overlaps with .bsdata
  >>> .header range is [0x2001EF, 0x20026B]
  >>> .bsdata range is [0x2001FF, 0x200398]

  ld.lld: error: section .bsdata file range overlaps with .entrytext
  >>> .bsdata range is [0x2001FF, 0x200398]
  >>> .entrytext range is [0x20026C, 0x2002D3]

  ld.lld: error: section .text.startup virtual address range overlaps
  with .header
  >>> .text.startup range is [0x40, 0x1FE]
  >>> .header range is [0x1EF, 0x26B]

  ld.lld: error: section .header virtual address range overlaps with
  .bsdata
  >>> .header range is [0x1EF, 0x26B]
  >>> .bsdata range is [0x1FF, 0x398]

  ld.lld: error: section .bsdata virtual address range overlaps with
  .entrytext
  >>> .bsdata range is [0x1FF, 0x398]
  >>> .entrytext range is [0x26C, 0x2D3]

  ld.lld: error: section .text.startup load address range overlaps with
  .header
  >>> .text.startup range is [0x40, 0x1FE]
  >>> .header range is [0x1EF, 0x26B]

  ld.lld: error: section .header load address range overlaps with
  .bsdata
  >>> .header range is [0x1EF, 0x26B]
  >>> .bsdata range is [0x1FF, 0x398]

  ld.lld: error: section .bsdata load address range overlaps with
  .entrytext
  >>> .bsdata range is [0x1FF, 0x398]
  >>> .entrytext range is [0x26C, 0x2D3]

Add .text.* to the .text output section to fix this, and also prevent
any future surprises if the compiler decides to create other such
sections.

[1] https://reviews.llvm.org/D75225

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/setup.ld | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/boot/setup.ld b/arch/x86/boot/setup.ld
index 24c95522f231..49546c247ae2 100644
--- a/arch/x86/boot/setup.ld
+++ b/arch/x86/boot/setup.ld
@@ -20,7 +20,7 @@ SECTIONS
 	.initdata	: { *(.initdata) }
 	__end_init = .;
 
-	.text		: { *(.text) }
+	.text		: { *(.text .text.*) }
 	.text32		: { *(.text32) }
 
 	. = ALIGN(16);
-- 
2.26.2

[PATCH v2 2/4] x86/boot: Remove run-time relocations from .head.text code

[Arvind Sankar]

The assembly code in head_{32,64}.S, while meant to be
position-independent, generates run-time relocations because it uses
instructions such as
	leal	gdt(%edx), %eax
which make the assembler and linker think that the code is using %edx as
an index into gdt, and hence gdt needs to be relocated to its run-time
address.

On 32-bit, with lld Dmitry Golovin reports that this results in a
link-time error with default options (i.e. unless -z notext is
explicitly passed):
  LD      arch/x86/boot/compressed/vmlinux
ld.lld: error: can't create dynamic relocation R_386_32 against local
symbol in readonly segment; recompile object files with -fPIC or pass
'-Wl,-z,notext' to allow text relocations in the output

With the BFD linker, this generates a warning during the build, if
--warn-shared-textrel is enabled, which at least Gentoo enables by
default:
  LD      arch/x86/boot/compressed/vmlinux
ld: arch/x86/boot/compressed/head_32.o: warning: relocation in read-only section `.head.text'
ld: warning: creating a DT_TEXTREL in object

On 64-bit, it is not possible to link the kernel as -pie with lld, and
it is only possible with a BFD linker that supports -z noreloc-overflow,
i.e. versions >2.26. This is because these instructions cannot really be
relocated: the displacement field is only 32-bits wide, and thus cannot
be relocated for a 64-bit load address. The -z noreloc-overflow option
simply overrides the linker error, and results in R_X86_64_RELATIVE
relocations that apply a 64-bit relocation to a 32-bit field anyway.
This happens to work because nothing will process these run-time
relocations.

Start fixing this by removing relocations from .head.text:
- On 32-bit, use a base register that holds the address of the GOT and
  reference symbol addresses using @GOTOFF, i.e.
	leal	gdt@GOTOFF(%edx), %eax
- On 64-bit, most of the code can (and already does) use %rip-relative
  addressing, however the .code32 bits can't, and the 64-bit code also
  needs to reference symbol addresses as they will be after moving the
  compressed kernel to the end of the decompression buffer.
  For these cases, reference the symbols as an offset to startup_32 to
  avoid creating relocations, i.e.
  	leal	(gdt-startup_32)(%bp), %eax
  This only works in .head.text as the subtraction cannot be represented
  as a PC-relative relocation unless startup_32 is in the same section
  as the code. Move efi32_pe_entry into .head.text so that it can use
  the same method to avoid relocations.

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/head_32.S |  40 +++++------
 arch/x86/boot/compressed/head_64.S | 104 ++++++++++++++++++-----------
 2 files changed, 86 insertions(+), 58 deletions(-)

diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index dfa4131c65df..66657bb99aae 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -73,10 +73,10 @@ SYM_FUNC_START(startup_32)
 	leal	(BP_scratch+4)(%esi), %esp
 	call	1f
 1:	popl	%edx
-	subl	$1b, %edx
+	addl	$_GLOBAL_OFFSET_TABLE_+(.-1b), %edx
 
 	/* Load new GDT */
-	leal	gdt(%edx), %eax
+	leal	gdt@GOTOFF(%edx), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
@@ -89,14 +89,16 @@ SYM_FUNC_START(startup_32)
 	movl	%eax, %ss
 
 /*
- * %edx contains the address we are loaded at by the boot loader and %ebx
- * contains the address where we should move the kernel image temporarily
- * for safe in-place decompression. %ebp contains the address that the kernel
- * will be decompressed to.
+ * %edx contains the address we are loaded at by the boot loader (plus the
+ * offset to the GOT).  The below code calculates %ebx to be the address where
+ * we should move the kernel image temporarily for safe in-place decompression
+ * (again, plus the offset to the GOT).
+ *
+ * %ebp is calculated to be the address that the kernel will be decompressed to.
  */
 
 #ifdef CONFIG_RELOCATABLE
-	movl	%edx, %ebx
+	leal	startup_32@GOTOFF(%edx), %ebx
 
 #ifdef CONFIG_EFI_STUB
 /*
@@ -107,7 +109,7 @@ SYM_FUNC_START(startup_32)
  *	image_offset = startup_32 - image_base
  * Otherwise image_offset will be zero and has no effect on the calculations.
  */
-	subl    image_offset(%edx), %ebx
+	subl    image_offset@GOTOFF(%edx), %ebx
 #endif
 
 	movl	BP_kernel_alignment(%esi), %eax
@@ -124,10 +126,10 @@ SYM_FUNC_START(startup_32)
 	movl	%ebx, %ebp	// Save the output address for later
 	/* Target address to relocate to for decompression */
 	addl    BP_init_size(%esi), %ebx
-	subl    $_end, %ebx
+	subl    $_end@GOTOFF, %ebx
 
 	/* Set up the stack */
-	leal	boot_stack_end(%ebx), %esp
+	leal	boot_stack_end@GOTOFF(%ebx), %esp
 
 	/* Zero EFLAGS */
 	pushl	$0
@@ -138,8 +140,8 @@ SYM_FUNC_START(startup_32)
  * where decompression in place becomes safe.
  */
 	pushl	%esi
-	leal	(_bss-4)(%edx), %esi
-	leal	(_bss-4)(%ebx), %edi
+	leal	(_bss@GOTOFF-4)(%edx), %esi
+	leal	(_bss@GOTOFF-4)(%ebx), %edi
 	movl	$(_bss - startup_32), %ecx
 	shrl	$2, %ecx
 	std
@@ -152,14 +154,14 @@ SYM_FUNC_START(startup_32)
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
 	 * to the new copy of the GDT.
 	 */
-	leal	gdt(%ebx), %eax
+	leal	gdt@GOTOFF(%ebx), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
 /*
  * Jump to the relocated address.
  */
-	leal	.Lrelocated(%ebx), %eax
+	leal	.Lrelocated@GOTOFF(%ebx), %eax
 	jmp	*%eax
 SYM_FUNC_END(startup_32)
 
@@ -169,7 +171,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
 	add	$0x4, %esp
 	movl	8(%esp), %esi	/* save boot_params pointer */
 	call	efi_main
-	leal	startup_32(%eax), %eax
+	/* efi_main returns the possibly relocated address of startup_32 */
 	jmp	*%eax
 SYM_FUNC_END(efi32_stub_entry)
 SYM_FUNC_END_ALIAS(efi_stub_entry)
@@ -182,8 +184,8 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
  * Clear BSS (stack is currently empty)
  */
 	xorl	%eax, %eax
-	leal	_bss(%ebx), %edi
-	leal	_ebss(%ebx), %ecx
+	leal	_bss@GOTOFF(%ebx), %edi
+	leal	_ebss@GOTOFF(%ebx), %ecx
 	subl	%edi, %ecx
 	shrl	$2, %ecx
 	rep	stosl
@@ -197,9 +199,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	pushl	%ebp		/* output address */
 
 	pushl	$z_input_len	/* input_len */
-	leal	input_data(%ebx), %eax
+	leal	input_data@GOTOFF(%ebx), %eax
 	pushl	%eax		/* input_data */
-	leal	boot_heap(%ebx), %eax
+	leal	boot_heap@GOTOFF(%ebx), %eax
 	pushl	%eax		/* heap area */
 	pushl	%esi		/* real mode pointer */
 	call	extract_kernel	/* returns kernel location in %eax */
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 706fbf6eef53..18b0edcb23d2 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -42,6 +42,32 @@
 	.hidden _ebss
 
 	__HEAD
+
+/*
+ * This macro gives the relative virtual address of X, i.e. the offset of X
+ * from startup_32. This is the same as the link-time virtual address of X,
+ * since startup_32 is at 0, but defining it this way tells the
+ * assembler/linker that we do not want the actual run-time address of X. This
+ * prevents the linker from trying to create unwanted run-time relocation
+ * entries for the reference when the compressed kernel is linked as PIE.
+ *
+ * A reference X(%reg) will result in the link-time VA of X being stored with
+ * the instruction, and a run-time R_X86_64_RELATIVE relocation entry that
+ * adds the 64-bit base address where the kernel is loaded.
+ *
+ * Replacing it with (X-startup_32)(%reg) results in the offset being stored,
+ * and no run-time relocation.
+ *
+ * The macro should be used as a displacement with a base register containing
+ * the run-time address of startup_32 [i.e. rva(X)(%reg)], or as an immediate
+ * [$ rva(X)].
+ *
+ * This macro can only be used from within the .head.text section, since the
+ * expression requires startup_32 to be in the same section as the code being
+ * assembled.
+ */
+#define rva(X) ((X) - startup_32)
+
 	.code32
 SYM_FUNC_START(startup_32)
 	/*
@@ -64,10 +90,10 @@ SYM_FUNC_START(startup_32)
 	leal	(BP_scratch+4)(%esi), %esp
 	call	1f
 1:	popl	%ebp
-	subl	$1b, %ebp
+	subl	$ rva(1b), %ebp
 
 	/* Load new GDT with the 64bit segments using 32bit descriptor */
-	leal	gdt(%ebp), %eax
+	leal	rva(gdt)(%ebp), %eax
 	movl	%eax, 2(%eax)
 	lgdt	(%eax)
 
@@ -80,7 +106,7 @@ SYM_FUNC_START(startup_32)
 	movl	%eax, %ss
 
 /* setup a stack and make sure cpu supports long mode. */
-	leal	boot_stack_end(%ebp), %esp
+	leal	rva(boot_stack_end)(%ebp), %esp
 
 	call	verify_cpu
 	testl	%eax, %eax
@@ -107,7 +133,7 @@ SYM_FUNC_START(startup_32)
  *	image_offset = startup_32 - image_base
  * Otherwise image_offset will be zero and has no effect on the calculations.
  */
-	subl    image_offset(%ebp), %ebx
+	subl    rva(image_offset)(%ebp), %ebx
 #endif
 
 	movl	BP_kernel_alignment(%esi), %eax
@@ -123,7 +149,7 @@ SYM_FUNC_START(startup_32)
 
 	/* Target address to relocate to for decompression */
 	addl	BP_init_size(%esi), %ebx
-	subl	$_end, %ebx
+	subl	$ rva(_end), %ebx
 
 /*
  * Prepare for entering 64 bit mode
@@ -151,19 +177,19 @@ SYM_FUNC_START(startup_32)
 1:
 
 	/* Initialize Page tables to 0 */
-	leal	pgtable(%ebx), %edi
+	leal	rva(pgtable)(%ebx), %edi
 	xorl	%eax, %eax
 	movl	$(BOOT_INIT_PGT_SIZE/4), %ecx
 	rep	stosl
 
 	/* Build Level 4 */
-	leal	pgtable + 0(%ebx), %edi
+	leal	rva(pgtable + 0)(%ebx), %edi
 	leal	0x1007 (%edi), %eax
 	movl	%eax, 0(%edi)
 	addl	%edx, 4(%edi)
 
 	/* Build Level 3 */
-	leal	pgtable + 0x1000(%ebx), %edi
+	leal	rva(pgtable + 0x1000)(%ebx), %edi
 	leal	0x1007(%edi), %eax
 	movl	$4, %ecx
 1:	movl	%eax, 0x00(%edi)
@@ -174,7 +200,7 @@ SYM_FUNC_START(startup_32)
 	jnz	1b
 
 	/* Build Level 2 */
-	leal	pgtable + 0x2000(%ebx), %edi
+	leal	rva(pgtable + 0x2000)(%ebx), %edi
 	movl	$0x00000183, %eax
 	movl	$2048, %ecx
 1:	movl	%eax, 0(%edi)
@@ -185,7 +211,7 @@ SYM_FUNC_START(startup_32)
 	jnz	1b
 
 	/* Enable the boot page tables */
-	leal	pgtable(%ebx), %eax
+	leal	rva(pgtable)(%ebx), %eax
 	movl	%eax, %cr3
 
 	/* Enable Long mode in EFER (Extended Feature Enable Register) */
@@ -211,17 +237,17 @@ SYM_FUNC_START(startup_32)
 	 * used to perform that far jump.
 	 */
 	pushl	$__KERNEL_CS
-	leal	startup_64(%ebp), %eax
+	leal	rva(startup_64)(%ebp), %eax
 #ifdef CONFIG_EFI_MIXED
-	movl	efi32_boot_args(%ebp), %edi
+	movl	rva(efi32_boot_args)(%ebp), %edi
 	cmp	$0, %edi
 	jz	1f
-	leal	efi64_stub_entry(%ebp), %eax
-	movl	efi32_boot_args+4(%ebp), %esi
-	movl	efi32_boot_args+8(%ebp), %edx	// saved bootparams pointer
+	leal	rva(efi64_stub_entry)(%ebp), %eax
+	movl	rva(efi32_boot_args+4)(%ebp), %esi
+	movl	rva(efi32_boot_args+8)(%ebp), %edx	// saved bootparams pointer
 	cmpl	$0, %edx
 	jnz	1f
-	leal	efi_pe_entry(%ebp), %eax
+	leal	rva(efi_pe_entry)(%ebp), %eax
 	movl	%edi, %ecx			// MS calling convention
 	movl	%esi, %edx
 1:
@@ -246,18 +272,18 @@ SYM_FUNC_START(efi32_stub_entry)
 
 	call	1f
 1:	pop	%ebp
-	subl	$1b, %ebp
+	subl	$ rva(1b), %ebp
 
-	movl	%esi, efi32_boot_args+8(%ebp)
+	movl	%esi, rva(efi32_boot_args+8)(%ebp)
 SYM_INNER_LABEL(efi32_pe_stub_entry, SYM_L_LOCAL)
-	movl	%ecx, efi32_boot_args(%ebp)
-	movl	%edx, efi32_boot_args+4(%ebp)
-	movb	$0, efi_is64(%ebp)
+	movl	%ecx, rva(efi32_boot_args)(%ebp)
+	movl	%edx, rva(efi32_boot_args+4)(%ebp)
+	movb	$0, rva(efi_is64)(%ebp)
 
 	/* Save firmware GDTR and code/data selectors */
-	sgdtl	efi32_boot_gdt(%ebp)
-	movw	%cs, efi32_boot_cs(%ebp)
-	movw	%ds, efi32_boot_ds(%ebp)
+	sgdtl	rva(efi32_boot_gdt)(%ebp)
+	movw	%cs, rva(efi32_boot_cs)(%ebp)
+	movw	%ds, rva(efi32_boot_ds)(%ebp)
 
 	/* Disable paging */
 	movl	%cr0, %eax
@@ -336,11 +362,11 @@ SYM_CODE_START(startup_64)
 
 	/* Target address to relocate to for decompression */
 	movl	BP_init_size(%rsi), %ebx
-	subl	$_end, %ebx
+	subl	$ rva(_end), %ebx
 	addq	%rbp, %rbx
 
 	/* Set up the stack */
-	leaq	boot_stack_end(%rbx), %rsp
+	leaq	rva(boot_stack_end)(%rbx), %rsp
 
 	/*
 	 * At this point we are in long mode with 4-level paging enabled,
@@ -406,7 +432,7 @@ SYM_CODE_START(startup_64)
 	lretq
 trampoline_return:
 	/* Restore the stack, the 32-bit trampoline uses its own stack */
-	leaq	boot_stack_end(%rbx), %rsp
+	leaq	rva(boot_stack_end)(%rbx), %rsp
 
 	/*
 	 * cleanup_trampoline() would restore trampoline memory.
@@ -418,7 +444,7 @@ trampoline_return:
 	 * this function call.
 	 */
 	pushq	%rsi
-	leaq	top_pgtable(%rbx), %rdi
+	leaq	rva(top_pgtable)(%rbx), %rdi
 	call	cleanup_trampoline
 	popq	%rsi
 
@@ -432,9 +458,9 @@ trampoline_return:
  */
 	pushq	%rsi
 	leaq	(_bss-8)(%rip), %rsi
-	leaq	(_bss-8)(%rbx), %rdi
-	movq	$_bss /* - $startup_32 */, %rcx
-	shrq	$3, %rcx
+	leaq	rva(_bss-8)(%rbx), %rdi
+	movl	$(_bss - startup_32), %ecx
+	shrl	$3, %ecx
 	std
 	rep	movsq
 	cld
@@ -445,15 +471,15 @@ trampoline_return:
 	 * during extract_kernel below. To avoid any issues, repoint the GDTR
 	 * to the new copy of the GDT.
 	 */
-	leaq	gdt64(%rbx), %rax
-	leaq	gdt(%rbx), %rdx
+	leaq	rva(gdt64)(%rbx), %rax
+	leaq	rva(gdt)(%rbx), %rdx
 	movq	%rdx, 2(%rax)
 	lgdt	(%rax)
 
 /*
  * Jump to the relocated address.
  */
-	leaq	.Lrelocated(%rbx), %rax
+	leaq	rva(.Lrelocated)(%rbx), %rax
 	jmp	*%rax
 SYM_CODE_END(startup_64)
 
@@ -465,7 +491,7 @@ SYM_FUNC_START_ALIAS(efi_stub_entry)
 	movq	%rdx, %rbx			/* save boot_params pointer */
 	call	efi_main
 	movq	%rbx,%rsi
-	leaq	startup_64(%rax), %rax
+	leaq	rva(startup_64)(%rax), %rax
 	jmp	*%rax
 SYM_FUNC_END(efi64_stub_entry)
 SYM_FUNC_END_ALIAS(efi_stub_entry)
@@ -628,7 +654,7 @@ SYM_DATA(efi_is64, .byte 1)
 #define BS32_handle_protocol	88 // offsetof(efi_boot_services_32_t, handle_protocol)
 #define LI32_image_base		32 // offsetof(efi_loaded_image_32_t, image_base)
 
-	.text
+	__HEAD
 	.code32
 SYM_FUNC_START(efi32_pe_entry)
 /*
@@ -650,12 +676,12 @@ SYM_FUNC_START(efi32_pe_entry)
 
 	call	1f
 1:	pop	%ebx
-	subl	$1b, %ebx
+	subl	$ rva(1b), %ebx
 
 	/* Get the loaded image protocol pointer from the image handle */
 	leal	-4(%ebp), %eax
 	pushl	%eax				// &loaded_image
-	leal	loaded_image_proto(%ebx), %eax
+	leal	rva(loaded_image_proto)(%ebx), %eax
 	pushl	%eax				// pass the GUID address
 	pushl	8(%ebp)				// pass the image handle
 
@@ -690,7 +716,7 @@ SYM_FUNC_START(efi32_pe_entry)
 	 * use it before we get to the 64-bit efi_pe_entry() in C code.
 	 */
 	subl	%esi, %ebx
-	movl	%ebx, image_offset(%ebp)	// save image_offset
+	movl	%ebx, rva(image_offset)(%ebp)	// save image_offset
 	jmp	efi32_pe_stub_entry
 
 2:	popl	%edi				// restore callee-save registers
-- 
2.26.2

[PATCH v2 3/4] x86/boot: Remove runtime relocations from head_{32,64}.S

[Arvind Sankar]

The BFD linker generates runtime relocations for z_input_len and
z_output_len, even though they are absolute symbols.

This is fixed for binutils-2.35 [1]. Work around this for earlier
versions by defining two variables input_len and output_len in addition
to the symbols, and use them via position-independent references.

This eliminates the last two runtime relocations in the head code and
allows us to drop the -z noreloc-overflow flag to the linker.

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=25754

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/Makefile  |  8 --------
 arch/x86/boot/compressed/head_32.S | 17 ++++++++---------
 arch/x86/boot/compressed/head_64.S |  4 ++--
 arch/x86/boot/compressed/mkpiggy.c |  6 ++++++
 4 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index aa9ed814e5fa..d3e882e855ee 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -49,15 +49,7 @@ UBSAN_SANITIZE :=n
 KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
 # Compressed kernel should be built as PIE since it may be loaded at any
 # address by the bootloader.
-ifeq ($(CONFIG_X86_32),y)
 KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
-else
-# To build 64-bit compressed kernel as PIE, we disable relocation
-# overflow check to avoid relocation overflow error with a new linker
-# command-line option, -z noreloc-overflow.
-KBUILD_LDFLAGS += $(shell $(LD) --help 2>&1 | grep -q "\-z noreloc-overflow" \
-	&& echo "-z noreloc-overflow -pie --no-dynamic-linker")
-endif
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 66657bb99aae..064e895bad92 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -193,18 +193,17 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 /*
  * Do the extraction, and jump to the new kernel..
  */
-				/* push arguments for extract_kernel: */
-	pushl	$z_output_len	/* decompressed length, end of relocs */
+	/* push arguments for extract_kernel: */
 
-	pushl	%ebp		/* output address */
-
-	pushl	$z_input_len	/* input_len */
+	pushl	output_len@GOTOFF(%ebx)	/* decompressed length, end of relocs */
+	pushl	%ebp			/* output address */
+	pushl	input_len@GOTOFF(%ebx)	/* input_len */
 	leal	input_data@GOTOFF(%ebx), %eax
-	pushl	%eax		/* input_data */
+	pushl	%eax			/* input_data */
 	leal	boot_heap@GOTOFF(%ebx), %eax
-	pushl	%eax		/* heap area */
-	pushl	%esi		/* real mode pointer */
-	call	extract_kernel	/* returns kernel location in %eax */
+	pushl	%eax			/* heap area */
+	pushl	%esi			/* real mode pointer */
+	call	extract_kernel		/* returns kernel location in %eax */
 	addl	$24, %esp
 
 /*
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 18b0edcb23d2..4b7ad1dfbea6 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -517,9 +517,9 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	movq	%rsi, %rdi		/* real mode address */
 	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
 	leaq	input_data(%rip), %rdx  /* input_data */
-	movl	$z_input_len, %ecx	/* input_len */
+	movl	input_len(%rip), %ecx	/* input_len */
 	movq	%rbp, %r8		/* output target address */
-	movl	$z_output_len, %r9d	/* decompressed length, end of relocs */
+	movl	output_len(%rip), %r9d	/* decompressed length, end of relocs */
 	call	extract_kernel		/* returns kernel location in %rax */
 	popq	%rsi
 
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index 7e01248765b2..52aa56cdbacc 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -60,6 +60,12 @@ int main(int argc, char *argv[])
 	printf(".incbin \"%s\"\n", argv[1]);
 	printf("input_data_end:\n");
 
+	printf(".section \".rodata\",\"a\",@progbits\n");
+	printf(".globl input_len\n");
+	printf("input_len:\n\t.long %lu\n", ilen);
+	printf(".globl output_len\n");
+	printf("output_len:\n\t.long %lu\n", (unsigned long)olen);
+
 	retval = 0;
 bail:
 	if (f)
-- 
2.26.2

[PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

Add a linker script check that there are no runtime relocations, and
remove the old one that tries to check via looking for specially-named
sections in the object files.

Drop the tests for -fPIE compiler option and -pie linker option, as they
are available in all supported gcc and binutils versions (as well as
clang and lld).

Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
 arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
 arch/x86/boot/compressed/vmlinux.lds.S |  8 ++++++++
 2 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index d3e882e855ee..679a2b383bfe 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
 	vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
 
 KBUILD_CFLAGS := -m$(BITS) -O2
-KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
+KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
 cflags-$(CONFIG_X86_32) := -march=i386
 cflags-$(CONFIG_X86_64) := -mcmodel=small
@@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
 KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
 # Compressed kernel should be built as PIE since it may be loaded at any
 # address by the bootloader.
-KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
+KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
 LDFLAGS_vmlinux := -T
 
 hostprogs	:= mkpiggy
@@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
 vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
 vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
 
-# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
-# can place it anywhere in memory and it will still run. However, since
-# it is executed as-is without any ELF relocation processing performed
-# (and has already had all relocation sections stripped from the binary),
-# none of the code can use data relocations (e.g. static assignments of
-# pointer values), since they will be meaningless at runtime. This check
-# will refuse to link the vmlinux if any of these relocations are found.
-quiet_cmd_check_data_rel = DATAREL $@
-define cmd_check_data_rel
-	for obj in $(filter %.o,$^); do \
-		$(READELF) -S $$obj | grep -qF .rel.local && { \
-			echo "error: $$obj has data relocations!" >&2; \
-			exit 1; \
-		} || true; \
-	done
-endef
-
-# We need to run two commands under "if_changed", so merge them into a
-# single invocation.
-quiet_cmd_check-and-link-vmlinux = LD      $@
-      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
-
 $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
-	$(call if_changed,check-and-link-vmlinux)
+	$(call if_changed,ld)
 
 OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
 $(obj)/vmlinux.bin: vmlinux FORCE
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index d826ab38a8f9..f9902c6ffe29 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -42,6 +42,12 @@ SECTIONS
 		*(.rodata.*)
 		_erodata = . ;
 	}
+	.rel.dyn : {
+		*(.rel.*)
+	}
+	.rela.dyn : {
+		*(.rela.*)
+	}
 	.got : {
 		*(.got)
 	}
@@ -83,3 +89,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
 #else
 ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
 #endif
+
+ASSERT(SIZEOF(.rel.dyn) == 0 && SIZEOF(.rela.dyn) == 0, "Unexpected runtime relocations detected!")
-- 
2.26.2

Re: [PATCH 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Fangrui Song]

On 2020-05-24, Arvind Sankar wrote:
>The compressed kernel currently contains bogus runtime relocations in
>the startup code in head_{32,64}.S, which are generated by the linker,
>but must not actually be processed at runtime.
>
>This generates warnings when linking with the BFD linker, and errors
>with LLD, which defaults to erroring on runtime relocations in read-only
>sections. It also requires the -z noreloc-overflow hack for the 64-bit
>kernel, which prevents us from linking it as -pie on an older BFD linker
>(<= 2.26) or on LLD, because the locations that are to be apparently
>relocated are only 32-bits in size and so cannot normally have
>R_X86_64_RELATIVE relocations.
>
>This series aims to get rid of these relocations. It is based on
>efi/next (efi-changes-for-v5.8), where the latest patches touch the
>head code to eliminate the global offset table.
>
>The first patch is an independent fix for LLD, to avoid an orphan
>section in arch/x86/boot/setup.elf [0].
>
>The second patch gets rid of almost all the relocations. It uses
>standard PIC addressing technique for 32-bit, i.e. loading a register
>with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
>references to access variables. For 64-bit, there is 32-bit code that
>cannot use RIP-relative addressing, and also cannot use the 32-bit
>method, since GOTOFF references are 64-bit only. This is instead handled
>using a macro to replace a reference like gdt with (gdt-startup_32)
>instead. The assembler will generate a PC32 relocation entry, with
>addend set to (.-startup_32), and these will be replaced with constants
>at link time. This works as long as all the code using such references
>lives in the same section as startup_32, i.e. in .head.text.
>
>The third patch addresses a remaining issue with the BFD linker, which
>insists on generating runtime relocations for absolute symbols. We use
>z_input_len and z_output_len, defined in the generated piggy.S file, as
>symbols whose absolute "addresses" are actually the size of the
>compressed payload and the size of the decompressed kernel image
>respectively. LLD does not generate relocations for these two symbols,
>but the BFD linker does. To get around this, piggy.S is extended to also
>define two u32 variables (in .rodata) with the lengths, and the head
>code is modified to use those instead of the symbol addresses.
>
>An alternative way to handle z_input_len/z_output_len would be to just
>include piggy.S in head_{32,64}.S instead of as a separate object file,
>since the GNU assembler doesn't generate relocations for symbols set to
>constants.
>
>The last patch adds a check in the linker script to ensure that no
>runtime relocations get reintroduced. Since the GOT has been eliminated
>as well, the compressed kernel has no runtime relocations whatsoever any
>more.
>
>[0] https://lore.kernel.org/lkml/20200521152459.558081-1-nivedita@alum.mit.edu/
>
>Arvind Sankar (4):
>  x86/boot: Add .text.startup to setup.ld
>  x86/boot: Remove runtime relocations from .head.text code
>  x86/boot: Remove runtime relocations from head_{32,64}.S
>  x86/boot: Check that there are no runtime relocations
>
> arch/x86/boot/compressed/Makefile      | 36 +---------
> arch/x86/boot/compressed/head_32.S     | 59 +++++++--------
> arch/x86/boot/compressed/head_64.S     | 99 +++++++++++++++-----------
> arch/x86/boot/compressed/mkpiggy.c     |  6 ++
> arch/x86/boot/compressed/vmlinux.lds.S | 11 +++
> arch/x86/boot/setup.ld                 |  2 +-
> 6 files changed, 109 insertions(+), 104 deletions(-)
>
>-- 
>2.26.2

All 4 commits look good.

Reviewed-by: Fangrui Song <maskray@google.com>

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Ard Biesheuvel]

On Tue, 26 May 2020 at 00:59, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> Add a linker script check that there are no runtime relocations, and
> remove the old one that tries to check via looking for specially-named
> sections in the object files.
>
> Drop the tests for -fPIE compiler option and -pie linker option, as they
> are available in all supported gcc and binutils versions (as well as
> clang and lld).
>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
>  arch/x86/boot/compressed/Makefile      | 28 +++-----------------------
>  arch/x86/boot/compressed/vmlinux.lds.S |  8 ++++++++
>  2 files changed, 11 insertions(+), 25 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index d3e882e855ee..679a2b383bfe 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -27,7 +27,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
>         vmlinux.bin.xz vmlinux.bin.lzo vmlinux.bin.lz4
>
>  KBUILD_CFLAGS := -m$(BITS) -O2
> -KBUILD_CFLAGS += -fno-strict-aliasing $(call cc-option, -fPIE, -fPIC)
> +KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
>  KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
>  cflags-$(CONFIG_X86_32) := -march=i386
>  cflags-$(CONFIG_X86_64) := -mcmodel=small
> @@ -49,7 +49,7 @@ UBSAN_SANITIZE :=n
>  KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
>  # Compressed kernel should be built as PIE since it may be loaded at any
>  # address by the bootloader.
> -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)

Do we still need -pie linking with these changes applied?

>  LDFLAGS_vmlinux := -T
>
>  hostprogs      := mkpiggy
> @@ -84,30 +84,8 @@ vmlinux-objs-$(CONFIG_ACPI) += $(obj)/acpi.o
>  vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
>  vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
>
> -# The compressed kernel is built with -fPIC/-fPIE so that a boot loader
> -# can place it anywhere in memory and it will still run. However, since
> -# it is executed as-is without any ELF relocation processing performed
> -# (and has already had all relocation sections stripped from the binary),
> -# none of the code can use data relocations (e.g. static assignments of
> -# pointer values), since they will be meaningless at runtime. This check
> -# will refuse to link the vmlinux if any of these relocations are found.
> -quiet_cmd_check_data_rel = DATAREL $@
> -define cmd_check_data_rel
> -       for obj in $(filter %.o,$^); do \
> -               $(READELF) -S $$obj | grep -qF .rel.local && { \
> -                       echo "error: $$obj has data relocations!" >&2; \
> -                       exit 1; \
> -               } || true; \
> -       done
> -endef
> -
> -# We need to run two commands under "if_changed", so merge them into a
> -# single invocation.
> -quiet_cmd_check-and-link-vmlinux = LD      $@
> -      cmd_check-and-link-vmlinux = $(cmd_check_data_rel); $(cmd_ld)
> -
>  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> -       $(call if_changed,check-and-link-vmlinux)
> +       $(call if_changed,ld)
>
>  OBJCOPYFLAGS_vmlinux.bin :=  -R .comment -S
>  $(obj)/vmlinux.bin: vmlinux FORCE
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index d826ab38a8f9..f9902c6ffe29 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -42,6 +42,12 @@ SECTIONS
>                 *(.rodata.*)
>                 _erodata = . ;
>         }
> +       .rel.dyn : {
> +               *(.rel.*)
> +       }
> +       .rela.dyn : {
> +               *(.rela.*)
> +       }
>         .got : {
>                 *(.got)
>         }
> @@ -83,3 +89,5 @@ ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, "Unexpected GOT/PLT en
>  #else
>  ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc, "Unexpected GOT/PLT entries detected!")
>  #endif
> +
> +ASSERT(SIZEOF(.rel.dyn) == 0 && SIZEOF(.rela.dyn) == 0, "Unexpected runtime relocations detected!")
> --
> 2.26.2
>

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> The compressed kernel currently contains bogus runtime relocations in
> the startup code in head_{32,64}.S, which are generated by the linker,
> but must not actually be processed at runtime.
>
> This generates warnings when linking with the BFD linker, and errors
> with LLD, which defaults to erroring on runtime relocations in read-only
> sections. It also requires the -z noreloc-overflow hack for the 64-bit
> kernel, which prevents us from linking it as -pie on an older BFD linker
> (<= 2.26) or on LLD, because the locations that are to be apparently
> relocated are only 32-bits in size and so cannot normally have
> R_X86_64_RELATIVE relocations.
>
> This series aims to get rid of these relocations. It is based on
> efi/next, where the latest patches touch the head code to eliminate the
> global offset table.
>
> The first patch is an independent fix for LLD, to avoid an orphan
> section in arch/x86/boot/setup.elf.
>
> The second patch gets rid of almost all the relocations. It uses
> standard PIC addressing technique for 32-bit, i.e. loading a register
> with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> references to access variables. For 64-bit, there is 32-bit code that
> cannot use RIP-relative addressing, and also cannot use the 32-bit
> method, since GOTOFF references are 64-bit only. This is instead handled
> using a macro to replace a reference like gdt with (gdt-startup_32)
> instead. The assembler will generate a PC32 relocation entry, with
> addend set to (.-startup_32), and these will be replaced with constants
> at link time. This works as long as all the code using such references
> lives in the same section as startup_32, i.e. in .head.text.
>
> The third patch addresses a remaining issue with the BFD linker, which
> insists on generating runtime relocations for absolute symbols. We use
> z_input_len and z_output_len, defined in the generated piggy.S file, as
> symbols whose absolute "addresses" are actually the size of the
> compressed payload and the size of the decompressed kernel image
> respectively. LLD does not generate relocations for these two symbols,
> but the BFD linker does, prior to the upcoming 2.35. To get around this,
> piggy.S is extended to also define two u32 variables (in .rodata) with
> the lengths, and the head code is modified to use those instead of the
> symbol addresses.
>
> An alternative way to handle z_input_len/z_output_len would be to just
> include piggy.S in head_{32,64}.S instead of as a separate object file,
> since the GNU assembler doesn't generate relocations for symbols set to
> constants.
>
> The last patch adds a check in the linker script to ensure that no
> runtime relocations get reintroduced. Since the GOT has been eliminated
> as well, the compressed kernel has no runtime relocations whatsoever any
> more.
>
> Changes from v1:
> - Add .text.* to setup.ld instead of just .text.startup
> - Rename the la() macro introduced in the second patch for 64-bit to
>   rva(), and rework the explanatory comment.
> - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
>   one per arch.
>

Hi,

I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:

[1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
[2] x86/boot: Correct relocation destination on old linkers

I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
merge problems, but I am not sure I did it correctly.
So, which patches are really relevant from efi/next?

What's your suggestions?

Regards,
- Sedat -

[1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/patch/?id=d6ee6529436a15a0541aff6e1697989ee7dc2c44
[2] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/patch/?id=5214028dd89e49ba27007c3ee475279e584261f0

> Arvind Sankar (4):
>   x86/boot: Add .text.* to setup.ld
>   x86/boot: Remove run-time relocations from .head.text code
>   x86/boot: Remove runtime relocations from head_{32,64}.S
>   x86/boot: Check that there are no runtime relocations
>
>  arch/x86/boot/compressed/Makefile      |  36 +--------
>  arch/x86/boot/compressed/head_32.S     |  59 +++++++-------
>  arch/x86/boot/compressed/head_64.S     | 108 +++++++++++++++----------
>  arch/x86/boot/compressed/mkpiggy.c     |   6 ++
>  arch/x86/boot/compressed/vmlinux.lds.S |   8 ++
>  arch/x86/boot/setup.ld                 |   2 +-
>  6 files changed, 115 insertions(+), 104 deletions(-)
>
> --
> 2.26.2
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200525225918.1624470-1-nivedita%40alum.mit.edu.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Ard Biesheuvel]

On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > The compressed kernel currently contains bogus runtime relocations in
> > the startup code in head_{32,64}.S, which are generated by the linker,
> > but must not actually be processed at runtime.
> >
> > This generates warnings when linking with the BFD linker, and errors
> > with LLD, which defaults to erroring on runtime relocations in read-only
> > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > kernel, which prevents us from linking it as -pie on an older BFD linker
> > (<= 2.26) or on LLD, because the locations that are to be apparently
> > relocated are only 32-bits in size and so cannot normally have
> > R_X86_64_RELATIVE relocations.
> >
> > This series aims to get rid of these relocations. It is based on
> > efi/next, where the latest patches touch the head code to eliminate the
> > global offset table.
> >
> > The first patch is an independent fix for LLD, to avoid an orphan
> > section in arch/x86/boot/setup.elf.
> >
> > The second patch gets rid of almost all the relocations. It uses
> > standard PIC addressing technique for 32-bit, i.e. loading a register
> > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > references to access variables. For 64-bit, there is 32-bit code that
> > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > method, since GOTOFF references are 64-bit only. This is instead handled
> > using a macro to replace a reference like gdt with (gdt-startup_32)
> > instead. The assembler will generate a PC32 relocation entry, with
> > addend set to (.-startup_32), and these will be replaced with constants
> > at link time. This works as long as all the code using such references
> > lives in the same section as startup_32, i.e. in .head.text.
> >
> > The third patch addresses a remaining issue with the BFD linker, which
> > insists on generating runtime relocations for absolute symbols. We use
> > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > symbols whose absolute "addresses" are actually the size of the
> > compressed payload and the size of the decompressed kernel image
> > respectively. LLD does not generate relocations for these two symbols,
> > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > piggy.S is extended to also define two u32 variables (in .rodata) with
> > the lengths, and the head code is modified to use those instead of the
> > symbol addresses.
> >
> > An alternative way to handle z_input_len/z_output_len would be to just
> > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > since the GNU assembler doesn't generate relocations for symbols set to
> > constants.
> >
> > The last patch adds a check in the linker script to ensure that no
> > runtime relocations get reintroduced. Since the GOT has been eliminated
> > as well, the compressed kernel has no runtime relocations whatsoever any
> > more.
> >
> > Changes from v1:
> > - Add .text.* to setup.ld instead of just .text.startup
> > - Rename the la() macro introduced in the second patch for 64-bit to
> >   rva(), and rework the explanatory comment.
> > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> >   one per arch.
> >
>
> Hi,
>
> I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
>
> [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> [2] x86/boot: Correct relocation destination on old linkers
>
> I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> merge problems, but I am not sure I did it correctly.
> So, which patches are really relevant from efi/next?
>
> What's your suggestions?
>

efi/next is here:

https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next

You'll need the top 3 patches.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > The compressed kernel currently contains bogus runtime relocations in
> > > the startup code in head_{32,64}.S, which are generated by the linker,
> > > but must not actually be processed at runtime.
> > >
> > > This generates warnings when linking with the BFD linker, and errors
> > > with LLD, which defaults to erroring on runtime relocations in read-only
> > > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > > kernel, which prevents us from linking it as -pie on an older BFD linker
> > > (<= 2.26) or on LLD, because the locations that are to be apparently
> > > relocated are only 32-bits in size and so cannot normally have
> > > R_X86_64_RELATIVE relocations.
> > >
> > > This series aims to get rid of these relocations. It is based on
> > > efi/next, where the latest patches touch the head code to eliminate the
> > > global offset table.
> > >
> > > The first patch is an independent fix for LLD, to avoid an orphan
> > > section in arch/x86/boot/setup.elf.
> > >
> > > The second patch gets rid of almost all the relocations. It uses
> > > standard PIC addressing technique for 32-bit, i.e. loading a register
> > > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > > references to access variables. For 64-bit, there is 32-bit code that
> > > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > > method, since GOTOFF references are 64-bit only. This is instead handled
> > > using a macro to replace a reference like gdt with (gdt-startup_32)
> > > instead. The assembler will generate a PC32 relocation entry, with
> > > addend set to (.-startup_32), and these will be replaced with constants
> > > at link time. This works as long as all the code using such references
> > > lives in the same section as startup_32, i.e. in .head.text.
> > >
> > > The third patch addresses a remaining issue with the BFD linker, which
> > > insists on generating runtime relocations for absolute symbols. We use
> > > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > > symbols whose absolute "addresses" are actually the size of the
> > > compressed payload and the size of the decompressed kernel image
> > > respectively. LLD does not generate relocations for these two symbols,
> > > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > > piggy.S is extended to also define two u32 variables (in .rodata) with
> > > the lengths, and the head code is modified to use those instead of the
> > > symbol addresses.
> > >
> > > An alternative way to handle z_input_len/z_output_len would be to just
> > > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > > since the GNU assembler doesn't generate relocations for symbols set to
> > > constants.
> > >
> > > The last patch adds a check in the linker script to ensure that no
> > > runtime relocations get reintroduced. Since the GOT has been eliminated
> > > as well, the compressed kernel has no runtime relocations whatsoever any
> > > more.
> > >
> > > Changes from v1:
> > > - Add .text.* to setup.ld instead of just .text.startup
> > > - Rename the la() macro introduced in the second patch for 64-bit to
> > >   rva(), and rework the explanatory comment.
> > > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> > >   one per arch.
> > >
> >
> > Hi,
> >
> > I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
> >
> > [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> > [2] x86/boot: Correct relocation destination on old linkers
> >
> > I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> > merge problems, but I am not sure I did it correctly.
> > So, which patches are really relevant from efi/next?
> >
> > What's your suggestions?
> >
>
> efi/next is here:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
>
> You'll need the top 3 patches.

Thanks /o\.

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 2:33 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > >
> > > On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > >
> > > > The compressed kernel currently contains bogus runtime relocations in
> > > > the startup code in head_{32,64}.S, which are generated by the linker,
> > > > but must not actually be processed at runtime.
> > > >
> > > > This generates warnings when linking with the BFD linker, and errors
> > > > with LLD, which defaults to erroring on runtime relocations in read-only
> > > > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > > > kernel, which prevents us from linking it as -pie on an older BFD linker
> > > > (<= 2.26) or on LLD, because the locations that are to be apparently
> > > > relocated are only 32-bits in size and so cannot normally have
> > > > R_X86_64_RELATIVE relocations.
> > > >
> > > > This series aims to get rid of these relocations. It is based on
> > > > efi/next, where the latest patches touch the head code to eliminate the
> > > > global offset table.
> > > >
> > > > The first patch is an independent fix for LLD, to avoid an orphan
> > > > section in arch/x86/boot/setup.elf.
> > > >
> > > > The second patch gets rid of almost all the relocations. It uses
> > > > standard PIC addressing technique for 32-bit, i.e. loading a register
> > > > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > > > references to access variables. For 64-bit, there is 32-bit code that
> > > > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > > > method, since GOTOFF references are 64-bit only. This is instead handled
> > > > using a macro to replace a reference like gdt with (gdt-startup_32)
> > > > instead. The assembler will generate a PC32 relocation entry, with
> > > > addend set to (.-startup_32), and these will be replaced with constants
> > > > at link time. This works as long as all the code using such references
> > > > lives in the same section as startup_32, i.e. in .head.text.
> > > >
> > > > The third patch addresses a remaining issue with the BFD linker, which
> > > > insists on generating runtime relocations for absolute symbols. We use
> > > > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > > > symbols whose absolute "addresses" are actually the size of the
> > > > compressed payload and the size of the decompressed kernel image
> > > > respectively. LLD does not generate relocations for these two symbols,
> > > > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > > > piggy.S is extended to also define two u32 variables (in .rodata) with
> > > > the lengths, and the head code is modified to use those instead of the
> > > > symbol addresses.
> > > >
> > > > An alternative way to handle z_input_len/z_output_len would be to just
> > > > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > > > since the GNU assembler doesn't generate relocations for symbols set to
> > > > constants.
> > > >
> > > > The last patch adds a check in the linker script to ensure that no
> > > > runtime relocations get reintroduced. Since the GOT has been eliminated
> > > > as well, the compressed kernel has no runtime relocations whatsoever any
> > > > more.
> > > >
> > > > Changes from v1:
> > > > - Add .text.* to setup.ld instead of just .text.startup
> > > > - Rename the la() macro introduced in the second patch for 64-bit to
> > > >   rva(), and rework the explanatory comment.
> > > > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> > > >   one per arch.
> > > >
> > >
> > > Hi,
> > >
> > > I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
> > >
> > > [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> > > [2] x86/boot: Correct relocation destination on old linkers
> > >
> > > I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> > > merge problems, but I am not sure I did it correctly.
> > > So, which patches are really relevant from efi/next?
> > >
> > > What's your suggestions?
> > >
> >
> > efi/next is here:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
> >
> > You'll need the top 3 patches.
>
> Thanks /o\.
>
> - Sedat -

Are those diffs correct when using "x86/boot: Correct relocation
destination on old linkers"?

$ cat ../head_32_S.diff
diff --cc arch/x86/boot/compressed/head_32.S
index 064e895bad92,03557f2174bf..000000000000
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@@ -49,13 -49,17 +49,14 @@@
   * Position Independent Executable (PIE) so that linker won't optimize
   * R_386_GOT32X relocation to its fixed symbol address.  Older
   * linkers generate R_386_32 relocations against locally defined symbols,
-  * _bss, _ebss, in PIE.  It isn't wrong, just suboptimal compared
 - * _bss, _ebss, _got, _egot and _end, in PIE.  It isn't wrong, just less
 - * optimal than R_386_RELATIVE.  But the x86 kernel fails to properly handle
++ * _bss, _ebss, _end in PIE.  It isn't wrong, just suboptimal compared
 + * to R_386_RELATIVE.  But the x86 kernel fails to properly handle
   * R_386_32 relocations when relocating the kernel.  To generate
-  * R_386_RELATIVE relocations, we mark _bss and _ebss as hidden:
 - * R_386_RELATIVE relocations, we mark _bss, _ebss, _got, _egot and _end as
 - * hidden:
++ * R_386_RELATIVE relocations, we mark _bss, _ebss and _end as hidden:
   */
        .hidden _bss
        .hidden _ebss
 -      .hidden _got
 -      .hidden _egot
+       .hidden _end

        __HEAD
  SYM_FUNC_START(startup_32)

$ cat ../head_64_S.diff
diff --cc arch/x86/boot/compressed/head_64.S
index 4b7ad1dfbea6,76d1d64d51e3..000000000000
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@@ -40,34 -40,11 +40,35 @@@
   */
        .hidden _bss
        .hidden _ebss
 -      .hidden _got
 -      .hidden _egot
+       .hidden _end

        __HEAD
 +
 +/*
 + * This macro gives the relative virtual address of X, i.e. the offset of X
 + * from startup_32. This is the same as the link-time virtual address of X,
 + * since startup_32 is at 0, but defining it this way tells the
 + * assembler/linker that we do not want the actual run-time address of X. This
 + * prevents the linker from trying to create unwanted run-time relocation
 + * entries for the reference when the compressed kernel is linked as PIE.
 + *
 + * A reference X(%reg) will result in the link-time VA of X being stored with
 + * the instruction, and a run-time R_X86_64_RELATIVE relocation entry that
 + * adds the 64-bit base address where the kernel is loaded.
 + *
 + * Replacing it with (X-startup_32)(%reg) results in the offset being stored,
 + * and no run-time relocation.
 + *
 + * The macro should be used as a displacement with a base register containing
 + * the run-time address of startup_32 [i.e. rva(X)(%reg)], or as an immediate
 + * [$ rva(X)].
 + *
 + * This macro can only be used from within the .head.text section, since the
 + * expression requires startup_32 to be in the same section as the code being
 + * assembled.
 + */
 +#define rva(X) ((X) - startup_32)
 +
        .code32
  SYM_FUNC_START(startup_32)
        /*

Thanks.

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Arvind Sankar]

On Tue, May 26, 2020 at 02:44:29PM +0200, Sedat Dilek wrote:
> 
> Are those diffs correct when using "x86/boot: Correct relocation
> destination on old linkers"?
> 

It looks ok, but that patch (and even marking the other symbols .hidden)
should be unnecessary after this series.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 2:44 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 2:33 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Tue, May 26, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > > >
> > > > On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > > >
> > > > > The compressed kernel currently contains bogus runtime relocations in
> > > > > the startup code in head_{32,64}.S, which are generated by the linker,
> > > > > but must not actually be processed at runtime.
> > > > >
> > > > > This generates warnings when linking with the BFD linker, and errors
> > > > > with LLD, which defaults to erroring on runtime relocations in read-only
> > > > > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > > > > kernel, which prevents us from linking it as -pie on an older BFD linker
> > > > > (<= 2.26) or on LLD, because the locations that are to be apparently
> > > > > relocated are only 32-bits in size and so cannot normally have
> > > > > R_X86_64_RELATIVE relocations.
> > > > >
> > > > > This series aims to get rid of these relocations. It is based on
> > > > > efi/next, where the latest patches touch the head code to eliminate the
> > > > > global offset table.
> > > > >
> > > > > The first patch is an independent fix for LLD, to avoid an orphan
> > > > > section in arch/x86/boot/setup.elf.
> > > > >
> > > > > The second patch gets rid of almost all the relocations. It uses
> > > > > standard PIC addressing technique for 32-bit, i.e. loading a register
> > > > > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > > > > references to access variables. For 64-bit, there is 32-bit code that
> > > > > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > > > > method, since GOTOFF references are 64-bit only. This is instead handled
> > > > > using a macro to replace a reference like gdt with (gdt-startup_32)
> > > > > instead. The assembler will generate a PC32 relocation entry, with
> > > > > addend set to (.-startup_32), and these will be replaced with constants
> > > > > at link time. This works as long as all the code using such references
> > > > > lives in the same section as startup_32, i.e. in .head.text.
> > > > >
> > > > > The third patch addresses a remaining issue with the BFD linker, which
> > > > > insists on generating runtime relocations for absolute symbols. We use
> > > > > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > > > > symbols whose absolute "addresses" are actually the size of the
> > > > > compressed payload and the size of the decompressed kernel image
> > > > > respectively. LLD does not generate relocations for these two symbols,
> > > > > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > > > > piggy.S is extended to also define two u32 variables (in .rodata) with
> > > > > the lengths, and the head code is modified to use those instead of the
> > > > > symbol addresses.
> > > > >
> > > > > An alternative way to handle z_input_len/z_output_len would be to just
> > > > > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > > > > since the GNU assembler doesn't generate relocations for symbols set to
> > > > > constants.
> > > > >
> > > > > The last patch adds a check in the linker script to ensure that no
> > > > > runtime relocations get reintroduced. Since the GOT has been eliminated
> > > > > as well, the compressed kernel has no runtime relocations whatsoever any
> > > > > more.
> > > > >
> > > > > Changes from v1:
> > > > > - Add .text.* to setup.ld instead of just .text.startup
> > > > > - Rename the la() macro introduced in the second patch for 64-bit to
> > > > >   rva(), and rework the explanatory comment.
> > > > > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> > > > >   one per arch.
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
> > > >
> > > > [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> > > > [2] x86/boot: Correct relocation destination on old linkers
> > > >
> > > > I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> > > > merge problems, but I am not sure I did it correctly.
> > > > So, which patches are really relevant from efi/next?
> > > >
> > > > What's your suggestions?
> > > >
> > >
> > > efi/next is here:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
> > >
> > > You'll need the top 3 patches.
> >
> > Thanks /o\.
> >
> > - Sedat -
>
> Are those diffs correct when using "x86/boot: Correct relocation
> destination on old linkers"?
>
> $ cat ../head_32_S.diff
> diff --cc arch/x86/boot/compressed/head_32.S
> index 064e895bad92,03557f2174bf..000000000000
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@@ -49,13 -49,17 +49,14 @@@
>    * Position Independent Executable (PIE) so that linker won't optimize
>    * R_386_GOT32X relocation to its fixed symbol address.  Older
>    * linkers generate R_386_32 relocations against locally defined symbols,
> -  * _bss, _ebss, in PIE.  It isn't wrong, just suboptimal compared
>  - * _bss, _ebss, _got, _egot and _end, in PIE.  It isn't wrong, just less
>  - * optimal than R_386_RELATIVE.  But the x86 kernel fails to properly handle
> ++ * _bss, _ebss, _end in PIE.  It isn't wrong, just suboptimal compared
>  + * to R_386_RELATIVE.  But the x86 kernel fails to properly handle
>    * R_386_32 relocations when relocating the kernel.  To generate
> -  * R_386_RELATIVE relocations, we mark _bss and _ebss as hidden:
>  - * R_386_RELATIVE relocations, we mark _bss, _ebss, _got, _egot and _end as
>  - * hidden:
> ++ * R_386_RELATIVE relocations, we mark _bss, _ebss and _end as hidden:
>    */
>         .hidden _bss
>         .hidden _ebss
>  -      .hidden _got
>  -      .hidden _egot
> +       .hidden _end
>
>         __HEAD
>   SYM_FUNC_START(startup_32)
>
> $ cat ../head_64_S.diff
> diff --cc arch/x86/boot/compressed/head_64.S
> index 4b7ad1dfbea6,76d1d64d51e3..000000000000
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@@ -40,34 -40,11 +40,35 @@@
>    */
>         .hidden _bss
>         .hidden _ebss
>  -      .hidden _got
>  -      .hidden _egot
> +       .hidden _end
>
>         __HEAD
>  +
>  +/*
>  + * This macro gives the relative virtual address of X, i.e. the offset of X
>  + * from startup_32. This is the same as the link-time virtual address of X,
>  + * since startup_32 is at 0, but defining it this way tells the
>  + * assembler/linker that we do not want the actual run-time address of X. This
>  + * prevents the linker from trying to create unwanted run-time relocation
>  + * entries for the reference when the compressed kernel is linked as PIE.
>  + *
>  + * A reference X(%reg) will result in the link-time VA of X being stored with
>  + * the instruction, and a run-time R_X86_64_RELATIVE relocation entry that
>  + * adds the 64-bit base address where the kernel is loaded.
>  + *
>  + * Replacing it with (X-startup_32)(%reg) results in the offset being stored,
>  + * and no run-time relocation.
>  + *
>  + * The macro should be used as a displacement with a base register containing
>  + * the run-time address of startup_32 [i.e. rva(X)(%reg)], or as an immediate
>  + * [$ rva(X)].
>  + *
>  + * This macro can only be used from within the .head.text section, since the
>  + * expression requires startup_32 to be in the same section as the code being
>  + * assembled.
>  + */
>  +#define rva(X) ((X) - startup_32)
>  +
>         .code32
>   SYM_FUNC_START(startup_32)
>         /*
>
> Thanks.
>

With LLVM/Clang/LLD I see:

  mycompiler -Wp,-MD,arch/x86/boot/compressed/.kernel_info.o.d
-nostdinc -isystem
/home/dileks/src/llvm-toolchain/install/lib/clang/10.0.1rc1/include
-I./arch/x86/include -I./arch/x86/include/generated  -I./include
-I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi
-I./include/uapi -I./include/generated/uapi -include
./include/linux/kconfig.h -D__KERNEL__ -Qunused-arguments -m64 -O2
-fno-strict-aliasing -fPIE -DDISABLE_BRANCH_PROFILING -mcmodel=small
-mno-mmx -mno-sse -ffreestanding -fno-stack-protector
-Wno-address-of-packed-member -Wno-gnu -Wno-pointer-sign
-fmacro-prefix-map=./= -fno-asynchronous-unwind-tables -include
hidden.h -D__ASSEMBLY__    -c -o
arch/x86/boot/compressed/kernel_info.o
arch/x86/boot/compressed/kernel_info.S
<built-in>:345:10: fatal error: 'hidden.h' file not found
#include "hidden.h"
         ^~~~~~~~~~
1 error generated.
make[5]: *** [scripts/Makefile.build:349:
arch/x86/boot/compressed/kernel_info.o] Error 1
make[4]: *** [arch/x86/boot/Makefile:114:
arch/x86/boot/compressed/vmlinux] Error 2
make[4]: *** Waiting for unfinished jobs....

mycompiler is a wrapper-script to use ccache * clang-10 as compiler.

patchset to previous build:

$ git log --no-merges --oneline 5.7.0-rc7-1-amd64-clang..5.7.0-rc7-2-amd64-clang
8b74901cb9e5 (for-5.7/x86-boot-remove-runtime-relocations-from-compressed-kernel-v2-nivedita76)
x86/boot: Check that there are no runtime relocations
83fb1bc3b076 x86/boot: Remove runtime relocations from head_{32,64}.S
fede23dacbbd x86/boot: Remove run-time relocations from .head.text code
3e5ea481b8fb x86/boot: Add .text.* to setup.ld
bec910ba3d67 x86/boot/compressed: Get rid of GOT fixup code
be834cee6f39 x86/boot/compressed: Force hidden visibility for all
symbol references
9b7c7d8d2d7b x86/boot/compressed: Move .got.plt entries out of the .got section
ba6a49f806a0 (for-5.7/kbuild-compressed-debug-info) Makefile: support
compressed debug info
a1fb11944d11 (for-5.7/x86-boot-nivedita76) x86/boot: Correct
relocation destination on old linkers
c70e3890058f (for-5.7/x86-build-maskray) x86/boot: Discard
.discard.unreachable for arch/x86/boot/compressed/vmlinux

$ find ./ -name hidden.h
./drivers/firmware/efi/libstub/hidden.h
./arch/x86/boot/compressed/hidden.h

Any thoughts?

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 4:47 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Tue, May 26, 2020 at 02:44:29PM +0200, Sedat Dilek wrote:
> >
> > Are those diffs correct when using "x86/boot: Correct relocation
> > destination on old linkers"?
> >
>
> It looks ok, but that patch (and even marking the other symbols .hidden)
> should be unnecessary after this series.

You mean _bss, _ebss and _end?

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 4:48 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 2:44 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Tue, May 26, 2020 at 2:33 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > >
> > > On Tue, May 26, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > >
> > > > On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > > > >
> > > > > On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > > > >
> > > > > > The compressed kernel currently contains bogus runtime relocations in
> > > > > > the startup code in head_{32,64}.S, which are generated by the linker,
> > > > > > but must not actually be processed at runtime.
> > > > > >
> > > > > > This generates warnings when linking with the BFD linker, and errors
> > > > > > with LLD, which defaults to erroring on runtime relocations in read-only
> > > > > > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > > > > > kernel, which prevents us from linking it as -pie on an older BFD linker
> > > > > > (<= 2.26) or on LLD, because the locations that are to be apparently
> > > > > > relocated are only 32-bits in size and so cannot normally have
> > > > > > R_X86_64_RELATIVE relocations.
> > > > > >
> > > > > > This series aims to get rid of these relocations. It is based on
> > > > > > efi/next, where the latest patches touch the head code to eliminate the
> > > > > > global offset table.
> > > > > >
> > > > > > The first patch is an independent fix for LLD, to avoid an orphan
> > > > > > section in arch/x86/boot/setup.elf.
> > > > > >
> > > > > > The second patch gets rid of almost all the relocations. It uses
> > > > > > standard PIC addressing technique for 32-bit, i.e. loading a register
> > > > > > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > > > > > references to access variables. For 64-bit, there is 32-bit code that
> > > > > > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > > > > > method, since GOTOFF references are 64-bit only. This is instead handled
> > > > > > using a macro to replace a reference like gdt with (gdt-startup_32)
> > > > > > instead. The assembler will generate a PC32 relocation entry, with
> > > > > > addend set to (.-startup_32), and these will be replaced with constants
> > > > > > at link time. This works as long as all the code using such references
> > > > > > lives in the same section as startup_32, i.e. in .head.text.
> > > > > >
> > > > > > The third patch addresses a remaining issue with the BFD linker, which
> > > > > > insists on generating runtime relocations for absolute symbols. We use
> > > > > > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > > > > > symbols whose absolute "addresses" are actually the size of the
> > > > > > compressed payload and the size of the decompressed kernel image
> > > > > > respectively. LLD does not generate relocations for these two symbols,
> > > > > > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > > > > > piggy.S is extended to also define two u32 variables (in .rodata) with
> > > > > > the lengths, and the head code is modified to use those instead of the
> > > > > > symbol addresses.
> > > > > >
> > > > > > An alternative way to handle z_input_len/z_output_len would be to just
> > > > > > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > > > > > since the GNU assembler doesn't generate relocations for symbols set to
> > > > > > constants.
> > > > > >
> > > > > > The last patch adds a check in the linker script to ensure that no
> > > > > > runtime relocations get reintroduced. Since the GOT has been eliminated
> > > > > > as well, the compressed kernel has no runtime relocations whatsoever any
> > > > > > more.
> > > > > >
> > > > > > Changes from v1:
> > > > > > - Add .text.* to setup.ld instead of just .text.startup
> > > > > > - Rename the la() macro introduced in the second patch for 64-bit to
> > > > > >   rva(), and rework the explanatory comment.
> > > > > > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> > > > > >   one per arch.
> > > > > >
> > > > >
> > > > > Hi,
> > > > >
> > > > > I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
> > > > >
> > > > > [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> > > > > [2] x86/boot: Correct relocation destination on old linkers
> > > > >
> > > > > I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> > > > > merge problems, but I am not sure I did it correctly.
> > > > > So, which patches are really relevant from efi/next?
> > > > >
> > > > > What's your suggestions?
> > > > >
> > > >
> > > > efi/next is here:
> > > >
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
> > > >
> > > > You'll need the top 3 patches.
> > >
> > > Thanks /o\.
> > >
> > > - Sedat -
> >
> > Are those diffs correct when using "x86/boot: Correct relocation
> > destination on old linkers"?
> >
> > $ cat ../head_32_S.diff
> > diff --cc arch/x86/boot/compressed/head_32.S
> > index 064e895bad92,03557f2174bf..000000000000
> > --- a/arch/x86/boot/compressed/head_32.S
> > +++ b/arch/x86/boot/compressed/head_32.S
> > @@@ -49,13 -49,17 +49,14 @@@
> >    * Position Independent Executable (PIE) so that linker won't optimize
> >    * R_386_GOT32X relocation to its fixed symbol address.  Older
> >    * linkers generate R_386_32 relocations against locally defined symbols,
> > -  * _bss, _ebss, in PIE.  It isn't wrong, just suboptimal compared
> >  - * _bss, _ebss, _got, _egot and _end, in PIE.  It isn't wrong, just less
> >  - * optimal than R_386_RELATIVE.  But the x86 kernel fails to properly handle
> > ++ * _bss, _ebss, _end in PIE.  It isn't wrong, just suboptimal compared
> >  + * to R_386_RELATIVE.  But the x86 kernel fails to properly handle
> >    * R_386_32 relocations when relocating the kernel.  To generate
> > -  * R_386_RELATIVE relocations, we mark _bss and _ebss as hidden:
> >  - * R_386_RELATIVE relocations, we mark _bss, _ebss, _got, _egot and _end as
> >  - * hidden:
> > ++ * R_386_RELATIVE relocations, we mark _bss, _ebss and _end as hidden:
> >    */
> >         .hidden _bss
> >         .hidden _ebss
> >  -      .hidden _got
> >  -      .hidden _egot
> > +       .hidden _end
> >
> >         __HEAD
> >   SYM_FUNC_START(startup_32)
> >
> > $ cat ../head_64_S.diff
> > diff --cc arch/x86/boot/compressed/head_64.S
> > index 4b7ad1dfbea6,76d1d64d51e3..000000000000
> > --- a/arch/x86/boot/compressed/head_64.S
> > +++ b/arch/x86/boot/compressed/head_64.S
> > @@@ -40,34 -40,11 +40,35 @@@
> >    */
> >         .hidden _bss
> >         .hidden _ebss
> >  -      .hidden _got
> >  -      .hidden _egot
> > +       .hidden _end
> >
> >         __HEAD
> >  +
> >  +/*
> >  + * This macro gives the relative virtual address of X, i.e. the offset of X
> >  + * from startup_32. This is the same as the link-time virtual address of X,
> >  + * since startup_32 is at 0, but defining it this way tells the
> >  + * assembler/linker that we do not want the actual run-time address of X. This
> >  + * prevents the linker from trying to create unwanted run-time relocation
> >  + * entries for the reference when the compressed kernel is linked as PIE.
> >  + *
> >  + * A reference X(%reg) will result in the link-time VA of X being stored with
> >  + * the instruction, and a run-time R_X86_64_RELATIVE relocation entry that
> >  + * adds the 64-bit base address where the kernel is loaded.
> >  + *
> >  + * Replacing it with (X-startup_32)(%reg) results in the offset being stored,
> >  + * and no run-time relocation.
> >  + *
> >  + * The macro should be used as a displacement with a base register containing
> >  + * the run-time address of startup_32 [i.e. rva(X)(%reg)], or as an immediate
> >  + * [$ rva(X)].
> >  + *
> >  + * This macro can only be used from within the .head.text section, since the
> >  + * expression requires startup_32 to be in the same section as the code being
> >  + * assembled.
> >  + */
> >  +#define rva(X) ((X) - startup_32)
> >  +
> >         .code32
> >   SYM_FUNC_START(startup_32)
> >         /*
> >
> > Thanks.
> >
>
> With LLVM/Clang/LLD I see:
>
>   mycompiler -Wp,-MD,arch/x86/boot/compressed/.kernel_info.o.d
> -nostdinc -isystem
> /home/dileks/src/llvm-toolchain/install/lib/clang/10.0.1rc1/include
> -I./arch/x86/include -I./arch/x86/include/generated  -I./include
> -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi
> -I./include/uapi -I./include/generated/uapi -include
> ./include/linux/kconfig.h -D__KERNEL__ -Qunused-arguments -m64 -O2
> -fno-strict-aliasing -fPIE -DDISABLE_BRANCH_PROFILING -mcmodel=small
> -mno-mmx -mno-sse -ffreestanding -fno-stack-protector
> -Wno-address-of-packed-member -Wno-gnu -Wno-pointer-sign
> -fmacro-prefix-map=./= -fno-asynchronous-unwind-tables -include
> hidden.h -D__ASSEMBLY__    -c -o
> arch/x86/boot/compressed/kernel_info.o
> arch/x86/boot/compressed/kernel_info.S
> <built-in>:345:10: fatal error: 'hidden.h' file not found
> #include "hidden.h"
>          ^~~~~~~~~~
> 1 error generated.
> make[5]: *** [scripts/Makefile.build:349:
> arch/x86/boot/compressed/kernel_info.o] Error 1
> make[4]: *** [arch/x86/boot/Makefile:114:
> arch/x86/boot/compressed/vmlinux] Error 2
> make[4]: *** Waiting for unfinished jobs....
>
> mycompiler is a wrapper-script to use ccache * clang-10 as compiler.
>
> patchset to previous build:
>
> $ git log --no-merges --oneline 5.7.0-rc7-1-amd64-clang..5.7.0-rc7-2-amd64-clang
> 8b74901cb9e5 (for-5.7/x86-boot-remove-runtime-relocations-from-compressed-kernel-v2-nivedita76)
> x86/boot: Check that there are no runtime relocations
> 83fb1bc3b076 x86/boot: Remove runtime relocations from head_{32,64}.S
> fede23dacbbd x86/boot: Remove run-time relocations from .head.text code
> 3e5ea481b8fb x86/boot: Add .text.* to setup.ld
> bec910ba3d67 x86/boot/compressed: Get rid of GOT fixup code
> be834cee6f39 x86/boot/compressed: Force hidden visibility for all
> symbol references
> 9b7c7d8d2d7b x86/boot/compressed: Move .got.plt entries out of the .got section
> ba6a49f806a0 (for-5.7/kbuild-compressed-debug-info) Makefile: support
> compressed debug info
> a1fb11944d11 (for-5.7/x86-boot-nivedita76) x86/boot: Correct
> relocation destination on old linkers
> c70e3890058f (for-5.7/x86-build-maskray) x86/boot: Discard
> .discard.unreachable for arch/x86/boot/compressed/vmlinux
>
> $ find ./ -name hidden.h
> ./drivers/firmware/efi/libstub/hidden.h
> ./arch/x86/boot/compressed/hidden.h
>
> Any thoughts?
>

Maybe this should be:

[ arch/x86/boot/compressed/Makefile ]

-KBUILD_CFLAGS += -include hidden.h
+KBUILD_CFLAGS += -include ./hidden.h

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 4:55 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Tue, May 26, 2020 at 4:48 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Tue, May 26, 2020 at 2:44 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > >
> > > On Tue, May 26, 2020 at 2:33 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > > >
> > > > On Tue, May 26, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >
> > > > > On Tue, 26 May 2020 at 14:29, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, May 26, 2020 at 12:59 AM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > > > > > >
> > > > > > > The compressed kernel currently contains bogus runtime relocations in
> > > > > > > the startup code in head_{32,64}.S, which are generated by the linker,
> > > > > > > but must not actually be processed at runtime.
> > > > > > >
> > > > > > > This generates warnings when linking with the BFD linker, and errors
> > > > > > > with LLD, which defaults to erroring on runtime relocations in read-only
> > > > > > > sections. It also requires the -z noreloc-overflow hack for the 64-bit
> > > > > > > kernel, which prevents us from linking it as -pie on an older BFD linker
> > > > > > > (<= 2.26) or on LLD, because the locations that are to be apparently
> > > > > > > relocated are only 32-bits in size and so cannot normally have
> > > > > > > R_X86_64_RELATIVE relocations.
> > > > > > >
> > > > > > > This series aims to get rid of these relocations. It is based on
> > > > > > > efi/next, where the latest patches touch the head code to eliminate the
> > > > > > > global offset table.
> > > > > > >
> > > > > > > The first patch is an independent fix for LLD, to avoid an orphan
> > > > > > > section in arch/x86/boot/setup.elf.
> > > > > > >
> > > > > > > The second patch gets rid of almost all the relocations. It uses
> > > > > > > standard PIC addressing technique for 32-bit, i.e. loading a register
> > > > > > > with the address of _GLOBAL_OFFSET_TABLE_ and then using GOTOFF
> > > > > > > references to access variables. For 64-bit, there is 32-bit code that
> > > > > > > cannot use RIP-relative addressing, and also cannot use the 32-bit
> > > > > > > method, since GOTOFF references are 64-bit only. This is instead handled
> > > > > > > using a macro to replace a reference like gdt with (gdt-startup_32)
> > > > > > > instead. The assembler will generate a PC32 relocation entry, with
> > > > > > > addend set to (.-startup_32), and these will be replaced with constants
> > > > > > > at link time. This works as long as all the code using such references
> > > > > > > lives in the same section as startup_32, i.e. in .head.text.
> > > > > > >
> > > > > > > The third patch addresses a remaining issue with the BFD linker, which
> > > > > > > insists on generating runtime relocations for absolute symbols. We use
> > > > > > > z_input_len and z_output_len, defined in the generated piggy.S file, as
> > > > > > > symbols whose absolute "addresses" are actually the size of the
> > > > > > > compressed payload and the size of the decompressed kernel image
> > > > > > > respectively. LLD does not generate relocations for these two symbols,
> > > > > > > but the BFD linker does, prior to the upcoming 2.35. To get around this,
> > > > > > > piggy.S is extended to also define two u32 variables (in .rodata) with
> > > > > > > the lengths, and the head code is modified to use those instead of the
> > > > > > > symbol addresses.
> > > > > > >
> > > > > > > An alternative way to handle z_input_len/z_output_len would be to just
> > > > > > > include piggy.S in head_{32,64}.S instead of as a separate object file,
> > > > > > > since the GNU assembler doesn't generate relocations for symbols set to
> > > > > > > constants.
> > > > > > >
> > > > > > > The last patch adds a check in the linker script to ensure that no
> > > > > > > runtime relocations get reintroduced. Since the GOT has been eliminated
> > > > > > > as well, the compressed kernel has no runtime relocations whatsoever any
> > > > > > > more.
> > > > > > >
> > > > > > > Changes from v1:
> > > > > > > - Add .text.* to setup.ld instead of just .text.startup
> > > > > > > - Rename the la() macro introduced in the second patch for 64-bit to
> > > > > > >   rva(), and rework the explanatory comment.
> > > > > > > - In the last patch, check both .rel.dyn and .rela.dyn, instead of just
> > > > > > >   one per arch.
> > > > > > >
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I would like to test this patchset v2 on top of Linux v5.7-rc7 together with:
> > > > > >
> > > > > > [1] x86/boot: Discard .discard.unreachable for arch/x86/boot/compressed/vmlinux
> > > > > > [2] x86/boot: Correct relocation destination on old linkers
> > > > > >
> > > > > > I tried to pull efi/next on top of Linux v5.7-rc7 and cleaned up the
> > > > > > merge problems, but I am not sure I did it correctly.
> > > > > > So, which patches are really relevant from efi/next?
> > > > > >
> > > > > > What's your suggestions?
> > > > > >
> > > > >
> > > > > efi/next is here:
> > > > >
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
> > > > >
> > > > > You'll need the top 3 patches.
> > > >
> > > > Thanks /o\.
> > > >
> > > > - Sedat -
> > >
> > > Are those diffs correct when using "x86/boot: Correct relocation
> > > destination on old linkers"?
> > >
> > > $ cat ../head_32_S.diff
> > > diff --cc arch/x86/boot/compressed/head_32.S
> > > index 064e895bad92,03557f2174bf..000000000000
> > > --- a/arch/x86/boot/compressed/head_32.S
> > > +++ b/arch/x86/boot/compressed/head_32.S
> > > @@@ -49,13 -49,17 +49,14 @@@
> > >    * Position Independent Executable (PIE) so that linker won't optimize
> > >    * R_386_GOT32X relocation to its fixed symbol address.  Older
> > >    * linkers generate R_386_32 relocations against locally defined symbols,
> > > -  * _bss, _ebss, in PIE.  It isn't wrong, just suboptimal compared
> > >  - * _bss, _ebss, _got, _egot and _end, in PIE.  It isn't wrong, just less
> > >  - * optimal than R_386_RELATIVE.  But the x86 kernel fails to properly handle
> > > ++ * _bss, _ebss, _end in PIE.  It isn't wrong, just suboptimal compared
> > >  + * to R_386_RELATIVE.  But the x86 kernel fails to properly handle
> > >    * R_386_32 relocations when relocating the kernel.  To generate
> > > -  * R_386_RELATIVE relocations, we mark _bss and _ebss as hidden:
> > >  - * R_386_RELATIVE relocations, we mark _bss, _ebss, _got, _egot and _end as
> > >  - * hidden:
> > > ++ * R_386_RELATIVE relocations, we mark _bss, _ebss and _end as hidden:
> > >    */
> > >         .hidden _bss
> > >         .hidden _ebss
> > >  -      .hidden _got
> > >  -      .hidden _egot
> > > +       .hidden _end
> > >
> > >         __HEAD
> > >   SYM_FUNC_START(startup_32)
> > >
> > > $ cat ../head_64_S.diff
> > > diff --cc arch/x86/boot/compressed/head_64.S
> > > index 4b7ad1dfbea6,76d1d64d51e3..000000000000
> > > --- a/arch/x86/boot/compressed/head_64.S
> > > +++ b/arch/x86/boot/compressed/head_64.S
> > > @@@ -40,34 -40,11 +40,35 @@@
> > >    */
> > >         .hidden _bss
> > >         .hidden _ebss
> > >  -      .hidden _got
> > >  -      .hidden _egot
> > > +       .hidden _end
> > >
> > >         __HEAD
> > >  +
> > >  +/*
> > >  + * This macro gives the relative virtual address of X, i.e. the offset of X
> > >  + * from startup_32. This is the same as the link-time virtual address of X,
> > >  + * since startup_32 is at 0, but defining it this way tells the
> > >  + * assembler/linker that we do not want the actual run-time address of X. This
> > >  + * prevents the linker from trying to create unwanted run-time relocation
> > >  + * entries for the reference when the compressed kernel is linked as PIE.
> > >  + *
> > >  + * A reference X(%reg) will result in the link-time VA of X being stored with
> > >  + * the instruction, and a run-time R_X86_64_RELATIVE relocation entry that
> > >  + * adds the 64-bit base address where the kernel is loaded.
> > >  + *
> > >  + * Replacing it with (X-startup_32)(%reg) results in the offset being stored,
> > >  + * and no run-time relocation.
> > >  + *
> > >  + * The macro should be used as a displacement with a base register containing
> > >  + * the run-time address of startup_32 [i.e. rva(X)(%reg)], or as an immediate
> > >  + * [$ rva(X)].
> > >  + *
> > >  + * This macro can only be used from within the .head.text section, since the
> > >  + * expression requires startup_32 to be in the same section as the code being
> > >  + * assembled.
> > >  + */
> > >  +#define rva(X) ((X) - startup_32)
> > >  +
> > >         .code32
> > >   SYM_FUNC_START(startup_32)
> > >         /*
> > >
> > > Thanks.
> > >
> >
> > With LLVM/Clang/LLD I see:
> >
> >   mycompiler -Wp,-MD,arch/x86/boot/compressed/.kernel_info.o.d
> > -nostdinc -isystem
> > /home/dileks/src/llvm-toolchain/install/lib/clang/10.0.1rc1/include
> > -I./arch/x86/include -I./arch/x86/include/generated  -I./include
> > -I./arch/x86/include/uapi -I./arch/x86/include/generated/uapi
> > -I./include/uapi -I./include/generated/uapi -include
> > ./include/linux/kconfig.h -D__KERNEL__ -Qunused-arguments -m64 -O2
> > -fno-strict-aliasing -fPIE -DDISABLE_BRANCH_PROFILING -mcmodel=small
> > -mno-mmx -mno-sse -ffreestanding -fno-stack-protector
> > -Wno-address-of-packed-member -Wno-gnu -Wno-pointer-sign
> > -fmacro-prefix-map=./= -fno-asynchronous-unwind-tables -include
> > hidden.h -D__ASSEMBLY__    -c -o
> > arch/x86/boot/compressed/kernel_info.o
> > arch/x86/boot/compressed/kernel_info.S
> > <built-in>:345:10: fatal error: 'hidden.h' file not found
> > #include "hidden.h"
> >          ^~~~~~~~~~
> > 1 error generated.
> > make[5]: *** [scripts/Makefile.build:349:
> > arch/x86/boot/compressed/kernel_info.o] Error 1
> > make[4]: *** [arch/x86/boot/Makefile:114:
> > arch/x86/boot/compressed/vmlinux] Error 2
> > make[4]: *** Waiting for unfinished jobs....
> >
> > mycompiler is a wrapper-script to use ccache * clang-10 as compiler.
> >
> > patchset to previous build:
> >
> > $ git log --no-merges --oneline 5.7.0-rc7-1-amd64-clang..5.7.0-rc7-2-amd64-clang
> > 8b74901cb9e5 (for-5.7/x86-boot-remove-runtime-relocations-from-compressed-kernel-v2-nivedita76)
> > x86/boot: Check that there are no runtime relocations
> > 83fb1bc3b076 x86/boot: Remove runtime relocations from head_{32,64}.S
> > fede23dacbbd x86/boot: Remove run-time relocations from .head.text code
> > 3e5ea481b8fb x86/boot: Add .text.* to setup.ld
> > bec910ba3d67 x86/boot/compressed: Get rid of GOT fixup code
> > be834cee6f39 x86/boot/compressed: Force hidden visibility for all
> > symbol references
> > 9b7c7d8d2d7b x86/boot/compressed: Move .got.plt entries out of the .got section
> > ba6a49f806a0 (for-5.7/kbuild-compressed-debug-info) Makefile: support
> > compressed debug info
> > a1fb11944d11 (for-5.7/x86-boot-nivedita76) x86/boot: Correct
> > relocation destination on old linkers
> > c70e3890058f (for-5.7/x86-build-maskray) x86/boot: Discard
> > .discard.unreachable for arch/x86/boot/compressed/vmlinux
> >
> > $ find ./ -name hidden.h
> > ./drivers/firmware/efi/libstub/hidden.h
> > ./arch/x86/boot/compressed/hidden.h
> >
> > Any thoughts?
> >
>
> Maybe this should be:
>
> [ arch/x86/boot/compressed/Makefile ]
>
> -KBUILD_CFLAGS += -include hidden.h
> +KBUILD_CFLAGS += -include ./hidden.h
>

NOPE.

This works:

[ arch/x86/boot/compressed/Makefile ]

-KBUILD_CFLAGS += -include hidden.h
+KBUILD_CFLAGS += -include ./arch/x86/boot/compressed/hidden.h

$ ll arch/x86/boot/bzImage arch/x86/boot/compressed/vmlinux
-rw-r--r-- 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/bzImage
-rwxr-xr-x 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/compressed/vmlinux

- Sedat -

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote:
> On Tue, 26 May 2020 at 00:59, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >  # Compressed kernel should be built as PIE since it may be loaded at any
> >  # address by the bootloader.
> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
> 
> Do we still need -pie linking with these changes applied?
> 

I think it's currently not strictly necessary -- eg the 64bit kernel
doesn't get linked as pie right now with LLD or old binutils. However,
it is safer to do so to ensure that the result remains PIC with future
versions of the linker. There are linker optimizations that can convert
certain PIC instructions when PIE is disabled. While I think they
currently all focus on eliminating indirection through the GOT (and thus
wouldn't be applicable any more), it's easy to imagine that they could
get extended to, for eg, convert
	leaq	foo(%rip), %rax
to
	movl	$foo, %eax
with some nop padding, etc.

Also, the relocation check that's being added here would only work with
PIE linking.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Arvind Sankar]

On Tue, May 26, 2020 at 05:07:24PM +0200, Sedat Dilek wrote:
> > >
> >
> > Maybe this should be:
> >
> > [ arch/x86/boot/compressed/Makefile ]
> >
> > -KBUILD_CFLAGS += -include hidden.h
> > +KBUILD_CFLAGS += -include ./hidden.h
> >
> 
> NOPE.
> 
> This works:
> 
> [ arch/x86/boot/compressed/Makefile ]
> 
> -KBUILD_CFLAGS += -include hidden.h
> +KBUILD_CFLAGS += -include ./arch/x86/boot/compressed/hidden.h
> 
> $ ll arch/x86/boot/bzImage arch/x86/boot/compressed/vmlinux
> -rw-r--r-- 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/bzImage
> -rwxr-xr-x 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/compressed/vmlinux
> 
> - Sedat -

It needs to either be $(srctree)/$(src)/hidden.h, or we should add
-I $(srctree)/$(src) to the KBUILD_CFLAGS. The latter option is added
automatically when building in a separate builddir with O=${KOBJ} (which
is how I, and I assume Ard, was testing), but for some reason is not
added when building in-tree. The -include option doesn't automatically
search the directory of the source file.

-include file Process file as if "#include "file"" appeared as the first
line of the primary source file.  However, the first directory searched
for file is the preprocessor's working directory instead of the
directory containing the main source file.  If not found there, it is
searched for in the remainder of the "#include "..."" search chain as
normal.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Arvind Sankar]

On Tue, May 26, 2020 at 04:50:38PM +0200, Sedat Dilek wrote:
> On Tue, May 26, 2020 at 4:47 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > On Tue, May 26, 2020 at 02:44:29PM +0200, Sedat Dilek wrote:
> > >
> > > Are those diffs correct when using "x86/boot: Correct relocation
> > > destination on old linkers"?
> > >
> >
> > It looks ok, but that patch (and even marking the other symbols .hidden)
> > should be unnecessary after this series.
> 
> You mean _bss, _ebss and _end?
> 
> - Sedat -

Yes. Those .hidden markings are there to ensure that when relocations
are generated (as they are currently), they're generated as
R_386_RELATIVE (which uses B+A calculation, with A being the link-time
virtual address of the symbol, and stored in the relocation field)
rather than R_386_32 (which uses S+A calculation, and so doesn't work
without runtime processing). After this patchset there aren't any
relocations, so while the .hidden markings won't hurt, they won't be
necessary either.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 5:36 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Tue, May 26, 2020 at 04:50:38PM +0200, Sedat Dilek wrote:
> > On Tue, May 26, 2020 at 4:47 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Tue, May 26, 2020 at 02:44:29PM +0200, Sedat Dilek wrote:
> > > >
> > > > Are those diffs correct when using "x86/boot: Correct relocation
> > > > destination on old linkers"?
> > > >
> > >
> > > It looks ok, but that patch (and even marking the other symbols .hidden)
> > > should be unnecessary after this series.
> >
> > You mean _bss, _ebss and _end?
> >
> > - Sedat -
>
> Yes. Those .hidden markings are there to ensure that when relocations
> are generated (as they are currently), they're generated as
> R_386_RELATIVE (which uses B+A calculation, with A being the link-time
> virtual address of the symbol, and stored in the relocation field)
> rather than R_386_32 (which uses S+A calculation, and so doesn't work
> without runtime processing). After this patchset there aren't any
> relocations, so while the .hidden markings won't hurt, they won't be
> necessary either.
>

So, I am here on Debian/testing AMD64.

How can I test the patchset worked correctly?

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 5:07 PM Sedat Dilek <sedat.dilek@gmail.com> wrote:

...

> > > patchset to previous build:
> > >
> > > $ git log --no-merges --oneline 5.7.0-rc7-1-amd64-clang..5.7.0-rc7-2-amd64-clang
> > > 8b74901cb9e5 (for-5.7/x86-boot-remove-runtime-relocations-from-compressed-kernel-v2-nivedita76)
> > > x86/boot: Check that there are no runtime relocations
> > > 83fb1bc3b076 x86/boot: Remove runtime relocations from head_{32,64}.S
> > > fede23dacbbd x86/boot: Remove run-time relocations from .head.text code
> > > 3e5ea481b8fb x86/boot: Add .text.* to setup.ld
> > > bec910ba3d67 x86/boot/compressed: Get rid of GOT fixup code
> > > be834cee6f39 x86/boot/compressed: Force hidden visibility for all
> > > symbol references
> > > 9b7c7d8d2d7b x86/boot/compressed: Move .got.plt entries out of the .got section
> > > ba6a49f806a0 (for-5.7/kbuild-compressed-debug-info) Makefile: support
> > > compressed debug info
> > > a1fb11944d11 (for-5.7/x86-boot-nivedita76) x86/boot: Correct
> > > relocation destination on old linkers
> > > c70e3890058f (for-5.7/x86-build-maskray) x86/boot: Discard
> > > .discard.unreachable for arch/x86/boot/compressed/vmlinux

...

> This works:
>
> [ arch/x86/boot/compressed/Makefile ]
>
> -KBUILD_CFLAGS += -include hidden.h
> +KBUILD_CFLAGS += -include ./arch/x86/boot/compressed/hidden.h
>
> $ ll arch/x86/boot/bzImage arch/x86/boot/compressed/vmlinux
> -rw-r--r-- 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/bzImage
> -rwxr-xr-x 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/compressed/vmlinux
>

I was able to build/link and boot on bare metal.

root@iniza:~# cat /proc/version
Linux version 5.7.0-rc7-2-amd64-clang (sedat.dilek@gmail.com@iniza)
(clang version 10.0.1rc1, LLD 10.0.1rc1) #2~bullseye+dileks1 SMP
2020-05-26

- Sedat -

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Fangrui Song]

On 2020-05-26, Arvind Sankar wrote:
>On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote:
>> On Tue, 26 May 2020 at 00:59, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>> >  # Compressed kernel should be built as PIE since it may be loaded at any
>> >  # address by the bootloader.
>> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
>> > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
>>
>> Do we still need -pie linking with these changes applied?
>>
>
>I think it's currently not strictly necessary -- eg the 64bit kernel
>doesn't get linked as pie right now with LLD or old binutils. However,
>it is safer to do so to ensure that the result remains PIC with future
>versions of the linker. There are linker optimizations that can convert
>certain PIC instructions when PIE is disabled. While I think they
>currently all focus on eliminating indirection through the GOT (and thus
>wouldn't be applicable any more),

There are 3 forms described by x86-64 psABI B.2 Optimize GOTPCRELX Relocations

(1) movq foo@GOTPCREL(%rip), %reg -> leaq foo(%rip), %reg
(2) call *foo@GOTPCREL(%rip) -> nop; call foo
(3) jmp *foo@GOTPCREL(%rip) -> jmp foo; nop

ld.bfd and gold perform (1) even for R_X86_64_GOTPCREL. LLD requires R_X86_64_[REX_]GOTPCRELX

>it's easy to imagine that they could
>get extended to, for eg, convert
>	leaq	foo(%rip), %rax
>to
>	movl	$foo, %eax
>with some nop padding, etc.

Not with NOP padding, but probably with instruction prefixes. It is
unclear the rewriting will be beneficial. Rewriting instructions definitely requires a
dedicated relocation type like R_X86_64_[REX_]GOTPCRELX.

>Also, the relocation check that's being added here would only work with
>PIE linking.

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

On Tue, May 26, 2020 at 10:13:40AM -0700, Fangrui Song wrote:
> 
> On 2020-05-26, Arvind Sankar wrote:
> >On Tue, May 26, 2020 at 08:11:56AM +0200, Ard Biesheuvel wrote:
> >> On Tue, 26 May 2020 at 00:59, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >> >  # Compressed kernel should be built as PIE since it may be loaded at any
> >> >  # address by the bootloader.
> >> > -KBUILD_LDFLAGS += $(call ld-option, -pie) $(call ld-option, --no-dynamic-linker)
> >> > +KBUILD_LDFLAGS += -pie $(call ld-option, --no-dynamic-linker)
> >>
> >> Do we still need -pie linking with these changes applied?
> >>
> >
> >I think it's currently not strictly necessary -- eg the 64bit kernel
> >doesn't get linked as pie right now with LLD or old binutils. However,
> >it is safer to do so to ensure that the result remains PIC with future
> >versions of the linker. There are linker optimizations that can convert
> >certain PIC instructions when PIE is disabled. While I think they
> >currently all focus on eliminating indirection through the GOT (and thus
> >wouldn't be applicable any more),
> 
> There are 3 forms described by x86-64 psABI B.2 Optimize GOTPCRELX Relocations
> 
> (1) movq foo@GOTPCREL(%rip), %reg -> leaq foo(%rip), %reg
> (2) call *foo@GOTPCREL(%rip) -> nop; call foo
> (3) jmp *foo@GOTPCREL(%rip) -> jmp foo; nop
> 
> ld.bfd and gold perform (1) even for R_X86_64_GOTPCREL. LLD requires R_X86_64_[REX_]GOTPCRELX

The psABI says (1) can be relaxed into mov $foo, %reg if PIC is disabled
and foo lives below 4Gb. Similarly for the "test and binop" cases. Such
a relaxation would produce code that's not PIC any more, and wouldn't
boot.

> 
> >it's easy to imagine that they could
> >get extended to, for eg, convert
> >	leaq	foo(%rip), %rax
> >to
> >	movl	$foo, %eax
> >with some nop padding, etc.
> 
> Not with NOP padding, but probably with instruction prefixes. It is
> unclear the rewriting will be beneficial. Rewriting instructions definitely requires a
> dedicated relocation type like R_X86_64_[REX_]GOTPCRELX.

It ought to be faster: according to Agner Fog's tables, upto 4x higher
throughput than the RIP-relative LEA, and movq $foo, %rax is actually
the same size.

To take a step back, there isn't any *point* in not specifying -pie
after these changes: it would be lying to the toolchain just for the
sake of lying. It is inherently fragile, and would work only because the
toolchain isn't sophisticated enough to do some optimizations.

Eg, consider that if you ask for the address of an external function,
the compiler will generate
	movq f@GOTPCREL(%rip), %reg
if f has default visibility, and
	leaq f(%rip), %reg
if f has hidden visibility.

If you then link without -pie, the former gets relaxed into the non-PIC
	movq $f, %reg
by the BFD linker, but the latter isn't relaxed. This is a missed
optimization, which happens because there's the GOTPCRELX to tell the
linker that the first form can be relaxed, and there's no special
relaxable relocation type for the second form.

The 64-bit kernel actually contains one of these relocations, prior to
Ard's patches to add hiddden visibility for everything. It currently
works with LLD (which can't use -pie) only because LLD doesn't appear to
perform the relaxation of
	movq f@GOTPCREL(%rip), %reg
all the way to
	movq $f, %reg
Binutils-2.34, which does do that relaxation, produces an unbootable
kernel if you leave out the -pie.

> 
> >Also, the relocation check that's being added here would only work with
> >PIE linking.

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 5:31 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Tue, May 26, 2020 at 05:07:24PM +0200, Sedat Dilek wrote:
> > > >
> > >
> > > Maybe this should be:
> > >
> > > [ arch/x86/boot/compressed/Makefile ]
> > >
> > > -KBUILD_CFLAGS += -include hidden.h
> > > +KBUILD_CFLAGS += -include ./hidden.h
> > >
> >
> > NOPE.
> >
> > This works:
> >
> > [ arch/x86/boot/compressed/Makefile ]
> >
> > -KBUILD_CFLAGS += -include hidden.h
> > +KBUILD_CFLAGS += -include ./arch/x86/boot/compressed/hidden.h
> >
> > $ ll arch/x86/boot/bzImage arch/x86/boot/compressed/vmlinux
> > -rw-r--r-- 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/bzImage
> > -rwxr-xr-x 1 dileks dileks 6,5M Mai 26 17:05 arch/x86/boot/compressed/vmlinux
> >
> > - Sedat -
>
> It needs to either be $(srctree)/$(src)/hidden.h, or we should add
> -I $(srctree)/$(src) to the KBUILD_CFLAGS. The latter option is added
> automatically when building in a separate builddir with O=${KOBJ} (which
> is how I, and I assume Ard, was testing), but for some reason is not
> added when building in-tree. The -include option doesn't automatically
> search the directory of the source file.
>
> -include file Process file as if "#include "file"" appeared as the first
> line of the primary source file.  However, the first directory searched
> for file is the preprocessor's working directory instead of the
> directory containing the main source file.  If not found there, it is
> searched for in the remainder of the "#include "..."" search chain as
> normal.

Will you send a follow-up or a v3 for this?

- Sedat -

Re: [PATCH v2 0/4] x86/boot: Remove runtime relocations from compressed kernel

[Sedat Dilek]

On Tue, May 26, 2020 at 5:36 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Tue, May 26, 2020 at 04:50:38PM +0200, Sedat Dilek wrote:
> > On Tue, May 26, 2020 at 4:47 PM Arvind Sankar <nivedita@alum.mit.edu> wrote:
> > >
> > > On Tue, May 26, 2020 at 02:44:29PM +0200, Sedat Dilek wrote:
> > > >
> > > > Are those diffs correct when using "x86/boot: Correct relocation
> > > > destination on old linkers"?
> > > >
> > >
> > > It looks ok, but that patch (and even marking the other symbols .hidden)
> > > should be unnecessary after this series.
> >
> > You mean _bss, _ebss and _end?
> >
> > - Sedat -
>
> Yes. Those .hidden markings are there to ensure that when relocations
> are generated (as they are currently), they're generated as
> R_386_RELATIVE (which uses B+A calculation, with A being the link-time
> virtual address of the symbol, and stored in the relocation field)
> rather than R_386_32 (which uses S+A calculation, and so doesn't work
> without runtime processing). After this patchset there aren't any
> relocations, so while the .hidden markings won't hurt, they won't be
> necessary either.
>

Do you plan a change on this?

- Sedat -

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Andy Shevchenko]

On Tue, May 26, 2020 at 03:14:11PM -0400, Arvind Sankar wrote:

Side question: are you going to submit a v3 of this?
Or i.o.w. what is the status of this series?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 4/4] x86/boot: Check that there are no runtime relocations

[Arvind Sankar]

On Thu, Aug 06, 2020 at 02:19:53PM +0300, Andy Shevchenko wrote:
> On Tue, May 26, 2020 at 03:14:11PM -0400, Arvind Sankar wrote:
> 
> Side question: are you going to submit a v3 of this?
> Or i.o.w. what is the status of this series?
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

The latest is v6 [0], which has some minor changes over this version and
is rebased on 5.8-rc7.

I will send out another rebase once the merge window closes.

Thanks.

[0] https://lore.kernel.org/lkml/20200731202738.2577854-1-nivedita@alum.mit.edu/