x86/boot: get rid of GOT entries and associated fixup code

Building position independent code using GCC by default results in references to symbols with external linkage to be resolved via GOT entries, which carry the absolute addresses of the symbols, and thus need to be corrected if the load time address of the executable != the link time address.

For fully linked binaries, such GOT indirected references are completely useless, and actually make the startup code more complicated than necessary, since these corrections may need to be applied more than once. In fact, we have been very careful to avoid such references in the EFI stub code, since it would require yet another [earlier] pass of GOT fixups which we currently don't implement.

Older GCCs were quirky when it came to overriding this behavior using symbol visibility, but now that we have increased the minimum GCC version to 4.6, we can actually start setting the symbol visibility to 'hidden' globally for all symbol references in the decompressor, getting rid of the GOT entirely. This means we can get rid of the GOT fixup code right away, and we can start using ordinary external symbol references in the EFI stub without running the risk of boot regressions.

CC'ing Linus and Maarten, who were involved in diagnosing an issue related to GOT entries emitted from the EFI stub ~5 years ago. [0] [1]

Many thanks to Arvind for the suggestions and the help in testing these changes. Tested on GCC 4.6 + binutils 2.24 (Ubuntu 14.04), and GCC 8 + binutils 2.31 (Debian Buster)

[0] https://lore.kernel.org/lkml/[email protected]/
[1] https://lore.kernel.org/lkml/CA+55aFxW9PmtjOf9nUQwpU8swsFqJOz8whZXcONo+XFmkSwezg@mail.gmail.com/

Ard Biesheuvel (3):
x86/boot/compressed: move .got.plt entries out of the .got section x86/boot/compressed: force hidden visibility for all symbol references x86/boot/compressed: get rid of GOT fixup code

 arch/x86/boot/compressed/Makefile      |  1 +
 arch/x86/boot/compressed/head_32.S     | 22 ++------
 arch/x86/boot/compressed/head_64.S     | 57 --------------------
 arch/x86/boot/compressed/hidden.h      | 19 +++++++
 arch/x86/boot/compressed/vmlinux.lds.S | 16 ++++--
 5 files changed, 36 insertions(+), 79 deletions(-)
 create mode 100644 arch/x86/boot/compressed/hidden.h

2.20.1

The .got.plt section contains the part of the GOT which is used by PLT entries, and which gets updated lazily by the dynamic loader when function calls are dispatched through those PLT entries.

On fully linked binaries such as the kernel proper or the decompressor, this never happens, and so in practice, the .got.plt section consists only of the first 3 magic entries that are meant to point at the _DYNAMIC section and at the fixup routine in the loader. However, since we don't use a dynamic loader, those entries are never populated or used.

This means that treating those entries like ordinary GOT entries, and fixing them up based on the actual placement of the executable in memory is completely pointless, and we can just ignore the .got.plt section entirely, provided that it has no additional entries beyond the first 3 ones.

So add an assertion in the linker script to ensure that this assumption holds, and move the contents out of the [_got, _egot) memory range that is modified by the GOT fixup routines.

---
 arch/x86/boot/compressed/vmlinux.lds.S | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

index 508cfa6828c5..51ca654e43a9 100644
@@ -44,10 +44,13 @@ SECTIONS
 	}
 	.got : {
 		_got = .;
-		KEEP(*(.got.plt))
 		KEEP(*(.got))
 		_egot = .;
 	}
+	.got.plt : {
+		KEEP(*(.got.plt))
+	}
+
 	.data :	{
 		_data = . ;
 		*(.data)

@@ -74,3 +77,11 @@ SECTIONS
 	. = ALIGN(PAGE_SIZE);	/* keep ZO size page aligned */
 	_end = .;
 }
+
+#ifdef CONFIG_X86_64
+ASSERT (SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18,
+	"Unexpected GOT/PLT entries detected!")
+#else
+ASSERT (SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0xc,
+	"Unexpected GOT/PLT entries detected!")
+#endif

2.20.1

Eliminate all GOT entries in the decompressor binary, by forcing hidden visibility for all symbol references, which informs the compiler that such references will be resolved at link time without the need for allocating GOT entries.

To ensure that no GOT entries will creep back in, add an assertion to the decompressor linker script that will fire if the .got section has a non-zero size.

---
 arch/x86/boot/compressed/Makefile      |  1 +
 arch/x86/boot/compressed/hidden.h      | 19 +++++++++++++++++++
 arch/x86/boot/compressed/vmlinux.lds.S |  1 +
 3 files changed, 21 insertions(+)

index 56aa5fa0a66b..361df91b2288 100644
@@ -39,6 +39,7 @@ KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
 KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
 KBUILD_CFLAGS += -Wno-pointer-sign
 KBUILD_CFLAGS += $(call cc-option,-fmacro-prefix-map=$(srctree)/=)
+KBUILD_CFLAGS += -include hidden.h
 
 KBUILD_AFLAGS  := $(KBUILD_CFLAGS) -D__ASSEMBLY__
 GCOV_PROFILE := n

new file mode 100644
index 000000000000..2a1693373cc7
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * When building position independent code with GCC using the -fPIC option,
+ * (or even the -fPIE one on older versions), it will assume that we are
+ * building a dynamic object (either a shared library or an executable) that
+ * may have symbol references that can only be resolved at load time. For a
+ * variety of reasons (ELF symbol preemption, the CoW footprint of the section
+ * that is modified by the loader), this results in all references to symbols
+ * with external linkage to go via entries in the Global Offset Table (GOT),
+ * which carries absolute addresses which need to be fixed up when the
+ * executable image is loaded at an offset which is different from its link
+ * time offset.
+ *
+ * Fortunately, there is a way to inform the compiler that such symbol
+ * references will be satisfied at link time rather than at load time, by
+ * giving them 'hidden' visibility.
+ */
+
+#pragma GCC visibility push(hidden)

index 51ca654e43a9..955496ff2fcd 100644
@@ -78,6 +78,7 @@ SECTIONS
 	_end = .;
 }
 
+ASSERT (SIZEOF(.got) == 0, "Unexpected GOT entries detected!")
 #ifdef CONFIG_X86_64
 ASSERT (SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18,
 	"Unexpected GOT/PLT entries detected!")

2.20.1

The EFI stub already sets the hidden visibility attribute for the few external symbol references that it contains, so it is not needed in the context of this series.

In the future, we can revisit this if we want to get rid of the various __pure getter functions, but that requires thorough testing on other architectures and toolchains, so I'd prefer to leave that for later.

We don't need it for the stub right now, but then this bit in the cover letter is not yet true, we still need to be careful about libstub code.

Spurious GOT entries will now be caught by the ASSERT() in the linker script and fail the build instead of causing hard to debug boot regressions, so it is not entirely untrue either. But I take your point.

In a previous patch, we have eliminated GOT entries from the decompressor binary and added an assertion that the .got section is empty. This means that the GOT fixup routines that exist in both the 32-bit and 64-bit startup routines have become dead code, and can be removed.

---
 arch/x86/boot/compressed/head_32.S     | 22 ++------
 arch/x86/boot/compressed/head_64.S     | 57 --------------------
 arch/x86/boot/compressed/vmlinux.lds.S |  2 -
 3 files changed, 3 insertions(+), 78 deletions(-)

index e43ac17cb9fb..b4dc883672a6 100644
@@ -49,16 +49,13 @@
  * 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, _got and _egot, in PIE.  It isn't wrong, just less
- * optimal than R_386_RELATIVE.  But the x86 kernel fails to properly handle
+ * _bss, _ebss, 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, _ebss, _got and _egot as
- * hidden:
+ * R_386_RELATIVE relocations, we mark _bss and _ebss as hidden:
  */
 	.hidden _bss
 	.hidden _ebss
-	.hidden _got
-	.hidden _egot
 
 	__HEAD
 SYM_FUNC_START(startup_32)

@@ -170,19 +167,6 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
 	shrl	$2, %ecx
 	rep	stosl
 
-/*
- * Adjust our own GOT
- */
-	leal	_got(%ebx), %edx
-	leal	_egot(%ebx), %ecx
-1:
-	cmpl	%ecx, %edx
-	jae	2f
-	addl	%ebx, (%edx)
-	addl	$4, %edx
-	jmp	1b
-2:
-
 /*
  * Do the extraction, and jump to the new kernel..
  */

index 955496ff2fcd..3fc308f94ce3 100644
@@ -43,9 +43,7 @@ SECTIONS
 		_erodata = . ;
 	}
 	.got : {
-		_got = .;
 		KEEP(*(.got))
-		_egot = .;
 	}
 	.got.plt : {
 		KEEP(*(.got.plt))

2.20.1