linux-kernel - [RFC PATCH] x86/boot/compressed: Detect data relocations at link time

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-Id: <20200109200904.514349-1-nivedita@alum.mit.edu>
Date:   Thu,  9 Jan 2020 15:09:04 -0500
From:   Arvind Sankar <nivedita@...m.mit.edu>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     Kees Cook <keescook@...omium.org>,
        "H . J . Lu" <hjl.tools@...il.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: [RFC PATCH] x86/boot/compressed: Detect data relocations at link time

98f78525371b ("x86/boot: Refuse to build with data relocations") checks
the .o files linked into compressed/vmlinux to see if any have a
*.rel.local section, which typically is created from a data relocation.

However, this check has some limitations:
- it doesn't check libstub, as that gets linked in as a .a file
- if the address of an external variable with default visibility is
  referenced, rather than static or hidden, the section doesn't have
  .local attached (i.e. it would be just .rel[a].data.rel for
  example)
- if the data is constant (eg const char * const table[] = { .. }) the
  section is .data.rel.ro[.local]

So it is dependent on how exactly the linker decides to name the
sections in various cases.

This patch modifies the linker script to capture all dynamic
relocations, except for those in .head.text and .text (which come from
head_{32,64}.o and are harmless), in .rel[a].bad and assert that those
sections are empty. This is still dependent on linker naming convention
of naming the final relocation sections as .rel[a]<section> but that
should be more stable than the intermediate ones created for object
files.

The last remaining data relocation, in head_64.o's gdt structure, is
also removed.

Signed-off-by: Arvind Sankar <nivedita@...m.mit.edu>

---
This patch is based on
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/?h=next
as of "efi/libstub/x86: use const attribute for efi_is_64bit()"
---
 arch/x86/boot/compressed/head_64.S     |  7 ++++---
 arch/x86/boot/compressed/vmlinux.lds.S | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 1f1f6c8139b3..1838b59c6d6a 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -121,8 +121,9 @@ SYM_FUNC_START(startup_32)
  */
 
 	/* Load new GDT with the 64bit segments using 32bit descriptor */
-	addl	%ebp, gdt+2(%ebp)
-	lgdt	gdt(%ebp)
+	leal	gdt(%ebp), %eax
+	movl	%eax, 2(%eax)
+	lgdt	(%eax)
 
 	/* Enable PAE mode */
 	movl	%cr4, %eax
@@ -619,7 +620,7 @@ SYM_DATA_END(gdt64)
 	.balign	8
 SYM_DATA_START_LOCAL(gdt)
 	.word	gdt_end - gdt
-	.long	gdt
+	.long	0
 	.word	0
 	.quad	0x00cf9a000000ffff	/* __KERNEL32_CS */
 	.quad	0x00af9a000000ffff	/* __KERNEL_CS */
diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
index 508cfa6828c5..1ba85b109ac0 100644
--- a/arch/x86/boot/compressed/vmlinux.lds.S
+++ b/arch/x86/boot/compressed/vmlinux.lds.S
@@ -73,4 +73,20 @@ SECTIONS
 #endif
 	. = ALIGN(PAGE_SIZE);	/* keep ZO size page aligned */
 	_end = .;
+
+	/* Discard text relocations */
+	/DISCARD/ : {
+		*(.rel.head.text .rel.text)
+		*(.rela.head.text .rela.text)
+	}
+
+	/* There should be no other relocations */
+	.rel.bad : {
+		*(.rel.*)
+	}
+	.rela.bad : {
+		*(.rela.*)
+	}
 }
+
+ASSERT (SIZEOF(.rel.bad) == 0 && SIZEOF(.rela.bad) == 0, "Compressed kernel has data relocations!");
-- 
2.24.1