2020-07-31 23:13:32

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 00/36] Warn on orphan section placement

https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/log/?h=linker/orphans/warn/v5

v5:
- rebase from -rc2 to -rc7 to avoid build failures on Clang vs binutils
- include Arvind's GOT fix-up series[3], since it touches many similar areas
- add PGO/AutoFDO section patch[4]
- split up x86 and arm changes into more digestable steps
- move several sections out of DISCARD and into zero-size asserts
- introduce COMMON_DISCARDS to improve ARM's linker scripts
v4: https://lore.kernel.org/lkml/[email protected]/
v3: https://lore.kernel.org/lkml/[email protected]/
v2: https://lore.kernel.org/lkml/[email protected]/
v1: https://lore.kernel.org/lkml/[email protected]/

A recent bug[1] was solved for builds linked with ld.lld, and tracking
it down took way longer than it needed to (a year). Ultimately, it
boiled down to differences between ld.bfd and ld.lld's handling of
orphan sections. Similar situation have continued to recur, and it's
clear the kernel build needs to be much more explicit about linker
sections. Similarly, the recent FGKASLR series brought up orphan section
handling too[2]. In all cases, it would have been nice if the linker was
running with --orphan-handling=warn so that surprise sections wouldn't
silently get mapped into the kernel image at locations up to the whim
of the linker's orphan handling logic. Instead, all desired sections
should be explicitly identified in the linker script (to be either kept,
discarded, or verified to be zero-sized) with any orphans throwing a
warning. The powerpc architecture has actually been doing this for some
time, so this series just extends that coverage to x86, arm, and arm64.

This has gotten sucecssful build testing under the following matrix:

compiler/linker: gcc+ld.bfd, clang+ld.lld
targets: defconfig, allmodconfig
architectures: x86, i386, arm64, arm
versions: v5.8-rc7, next-20200731 (with various build fixes[7][8])

Two known-failure exceptions (unchanged by this series) being:
- clang+arm/arm64 needs CONFIG_CPU_BIG_ENDIAN=n to pass allmodconfig[5]
- clang+i386 only builds in -next, which was recently fixed[6]

All three architectures depend on the first several commits to
vmlinux.lds.h. x86 depends on Arvind's GOT series[3], so I collected it
into this version of my series, as it hadn't been taken into -tip yet.
arm64 depends on the efi/libstub patch. As such, I'd like to land this
series as a whole. Given that two thirds of it is in the arm universe,
perhaps this can land via the arm64 tree? If x86 -tip is preferred, that
works too. If I don't hear otherwise, I will just carry this myself in
-next. In all cases, I would really appreciate reviews/acks/etc. :)

Thanks!

-Kees

[1] https://github.com/ClangBuiltLinux/linux/issues/282
[2] https://lore.kernel.org/lkml/202002242122.AA4D1B8@keescook/
[3] https://lore.kernel.org/lkml/[email protected]/
[4] https://lore.kernel.org/lkml/[email protected]/
[5] https://github.com/ClangBuiltLinux/linux/issues/1071
[6] https://github.com/ClangBuiltLinux/linux/issues/194
[7] https://lore.kernel.org/lkml/[email protected]/
[8] https://lore.kernel.org/lkml/[email protected]/


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

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

Kees Cook (28):
vmlinux.lds.h: Create COMMON_DISCARDS
vmlinux.lds.h: Add .gnu.version* to COMMON_DISCARDS
vmlinux.lds.h: Avoid KASAN and KCSAN's unwanted sections
vmlinux.lds.h: Split ELF_DETAILS from STABS_DEBUG
vmlinux.lds.h: Add .symtab, .strtab, and .shstrtab to ELF_DETAILS
efi/libstub: Disable -mbranch-protection
arm64/mm: Remove needless section quotes
arm64/kernel: Remove needless Call Frame Information annotations
arm64/build: Remove .eh_frame* sections due to unwind tables
arm64/build: Use common DISCARDS in linker script
arm64/build: Add missing DWARF sections
arm64/build: Assert for unwanted sections
arm64/build: Warn on orphan section placement
arm/build: Refactor linker script headers
arm/build: Explicitly keep .ARM.attributes sections
arm/build: Add missing sections
arm/build: Warn on orphan section placement
arm/boot: Handle all sections explicitly
arm/boot: Warn on orphan section placement
x86/asm: Avoid generating unused kprobe sections
x86/build: Enforce an empty .got.plt section
x86/build: Assert for unwanted sections
x86/build: Warn on orphan section placement
x86/boot/compressed: Reorganize zero-size section asserts
x86/boot/compressed: Remove, discard, or assert for unwanted sections
x86/boot/compressed: Add missing debugging sections to output
x86/boot/compressed: Warn on orphan section placement
arm/build: Assert for unwanted sections

Nick Desaulniers (1):
vmlinux.lds.h: add PGO and AutoFDO input sections

arch/alpha/kernel/vmlinux.lds.S | 1 +
arch/arc/kernel/vmlinux.lds.S | 1 +
arch/arm/Makefile | 4 +
arch/arm/boot/compressed/Makefile | 2 +
arch/arm/boot/compressed/vmlinux.lds.S | 20 +--
.../arm/{kernel => include/asm}/vmlinux.lds.h | 29 ++-
arch/arm/kernel/vmlinux-xip.lds.S | 8 +-
arch/arm/kernel/vmlinux.lds.S | 8 +-
arch/arm64/Makefile | 9 +-
arch/arm64/kernel/smccc-call.S | 2 -
arch/arm64/kernel/vmlinux.lds.S | 28 ++-
arch/arm64/mm/mmu.c | 2 +-
arch/csky/kernel/vmlinux.lds.S | 1 +
arch/hexagon/kernel/vmlinux.lds.S | 1 +
arch/ia64/kernel/vmlinux.lds.S | 1 +
arch/mips/kernel/vmlinux.lds.S | 1 +
arch/nds32/kernel/vmlinux.lds.S | 1 +
arch/nios2/kernel/vmlinux.lds.S | 1 +
arch/openrisc/kernel/vmlinux.lds.S | 1 +
arch/parisc/boot/compressed/vmlinux.lds.S | 1 +
arch/parisc/kernel/vmlinux.lds.S | 1 +
arch/powerpc/kernel/vmlinux.lds.S | 2 +-
arch/riscv/kernel/vmlinux.lds.S | 1 +
arch/s390/kernel/vmlinux.lds.S | 1 +
arch/sh/kernel/vmlinux.lds.S | 1 +
arch/sparc/kernel/vmlinux.lds.S | 1 +
arch/um/kernel/dyn.lds.S | 2 +-
arch/um/kernel/uml.lds.S | 2 +-
arch/unicore32/kernel/vmlinux.lds.S | 1 +
arch/x86/Makefile | 4 +
arch/x86/boot/compressed/Makefile | 41 +----
arch/x86/boot/compressed/head_32.S | 99 ++++-------
arch/x86/boot/compressed/head_64.S | 165 +++++++-----------
arch/x86/boot/compressed/mkpiggy.c | 6 +
arch/x86/boot/compressed/vmlinux.lds.S | 48 ++++-
arch/x86/boot/setup.ld | 2 +-
arch/x86/include/asm/asm.h | 6 +-
arch/x86/kernel/vmlinux.lds.S | 39 ++++-
drivers/firmware/efi/libstub/Makefile | 11 +-
drivers/firmware/efi/libstub/hidden.h | 6 -
include/asm-generic/vmlinux.lds.h | 49 +++++-
include/linux/hidden.h | 19 ++
42 files changed, 377 insertions(+), 252 deletions(-)
rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (84%)
delete mode 100644 drivers/firmware/efi/libstub/hidden.h
create mode 100644 include/linux/hidden.h

--
2.25.1


2020-07-31 23:18:53

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 15/36] arm64/mm: Remove needless section quotes

Fix a case of needless quotes in __section(), which Clang doesn't like.

Acked-by: Will Deacon <[email protected]>
Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm64/mm/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
index 1df25f26571d..dce024ea6084 100644
--- a/arch/arm64/mm/mmu.c
+++ b/arch/arm64/mm/mmu.c
@@ -42,7 +42,7 @@
u64 idmap_t0sz = TCR_T0SZ(VA_BITS);
u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;

-u64 __section(".mmuoff.data.write") vabits_actual;
+u64 __section(.mmuoff.data.write) vabits_actual;
EXPORT_SYMBOL(vabits_actual);

u64 kimage_voffset __ro_after_init;
--
2.25.1

2020-07-31 23:18:53

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 22/36] arm/build: Refactor linker script headers

In preparation for adding --orphan-handling=warn, refactor the linker
script header includes, and extract common macros.

Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/{kernel => include/asm}/vmlinux.lds.h | 13 ++++++++-----
arch/arm/kernel/vmlinux-xip.lds.S | 4 +---
arch/arm/kernel/vmlinux.lds.S | 4 +---
3 files changed, 10 insertions(+), 11 deletions(-)
rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (96%)

diff --git a/arch/arm/kernel/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
similarity index 96%
rename from arch/arm/kernel/vmlinux.lds.h
rename to arch/arm/include/asm/vmlinux.lds.h
index 381a8e105fa5..a08f4301b718 100644
--- a/arch/arm/kernel/vmlinux.lds.h
+++ b/arch/arm/include/asm/vmlinux.lds.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include <asm-generic/vmlinux.lds.h>

#ifdef CONFIG_HOTPLUG_CPU
#define ARM_CPU_DISCARD(x)
@@ -49,8 +50,12 @@
EXIT_CALL \
ARM_MMU_DISCARD(*(.text.fixup)) \
ARM_MMU_DISCARD(*(__ex_table)) \
- *(.discard) \
- *(.discard.*)
+ COMMON_DISCARDS
+
+#define ARM_STUBS_TEXT \
+ *(.gnu.warning) \
+ *(.glue_7) \
+ *(.glue_7t)

#define ARM_TEXT \
IDMAP_TEXT \
@@ -64,9 +69,7 @@
CPUIDLE_TEXT \
LOCK_TEXT \
KPROBES_TEXT \
- *(.gnu.warning) \
- *(.glue_7) \
- *(.glue_7t) \
+ ARM_STUBS_TEXT \
. = ALIGN(4); \
*(.got) /* Global offset table */ \
ARM_CPU_KEEP(PROC_INFO)
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
index 3d4e88f08196..904c31fa20ed 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -9,15 +9,13 @@

#include <linux/sizes.h>

-#include <asm-generic/vmlinux.lds.h>
+#include <asm/vmlinux.lds.h>
#include <asm/cache.h>
#include <asm/thread_info.h>
#include <asm/memory.h>
#include <asm/mpu.h>
#include <asm/page.h>

-#include "vmlinux.lds.h"
-
OUTPUT_ARCH(arm)
ENTRY(stext)

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 5592f14b7e35..bb950c896a67 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -9,15 +9,13 @@
#else

#include <linux/pgtable.h>
-#include <asm-generic/vmlinux.lds.h>
+#include <asm/vmlinux.lds.h>
#include <asm/cache.h>
#include <asm/thread_info.h>
#include <asm/memory.h>
#include <asm/mpu.h>
#include <asm/page.h>

-#include "vmlinux.lds.h"
-
OUTPUT_ARCH(arm)
ENTRY(stext)

--
2.25.1

2020-07-31 23:19:03

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 23/36] arm/build: Explicitly keep .ARM.attributes sections

In preparation for adding --orphan-handling=warn, explicitly keep the
.ARM.attributes section by expanding the existing ELF_DETAILS macro into
ARM_DETAILS.

Suggested-by: Nick Desaulniers <[email protected]>
Link: https://lore.kernel.org/lkml/CAKwvOdk-racgq5pxsoGS6Vtifbtrk5fmkmnoLxrQMaOvV0nPWw@mail.gmail.com/
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/include/asm/vmlinux.lds.h | 4 ++++
arch/arm/kernel/vmlinux-xip.lds.S | 2 +-
arch/arm/kernel/vmlinux.lds.S | 2 +-
3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
index a08f4301b718..c4af5182ab48 100644
--- a/arch/arm/include/asm/vmlinux.lds.h
+++ b/arch/arm/include/asm/vmlinux.lds.h
@@ -52,6 +52,10 @@
ARM_MMU_DISCARD(*(__ex_table)) \
COMMON_DISCARDS

+#define ARM_DETAILS \
+ ELF_DETAILS \
+ .ARM.attributes 0 : { *(.ARM.attributes) }
+
#define ARM_STUBS_TEXT \
*(.gnu.warning) \
*(.glue_7) \
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
index 904c31fa20ed..57fcbf55f913 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -150,7 +150,7 @@ SECTIONS
_end = .;

STABS_DEBUG
- ELF_DETAILS
+ ARM_DETAILS
}

/*
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index bb950c896a67..1d3d3b599635 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -149,7 +149,7 @@ SECTIONS
_end = .;

STABS_DEBUG
- ELF_DETAILS
+ ARM_DETAILS
}

#ifdef CONFIG_STRICT_KERNEL_RWX
--
2.25.1

2020-07-31 23:19:09

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 24/36] arm/build: Add missing sections

Add missing text stub sections .vfp11_veneer and .v4_bx, as well as
missing DWARF sections, when present in the build.

Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/include/asm/vmlinux.lds.h | 4 +++-
arch/arm/kernel/vmlinux-xip.lds.S | 1 +
arch/arm/kernel/vmlinux.lds.S | 1 +
3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
index c4af5182ab48..6624dd97475c 100644
--- a/arch/arm/include/asm/vmlinux.lds.h
+++ b/arch/arm/include/asm/vmlinux.lds.h
@@ -59,7 +59,9 @@
#define ARM_STUBS_TEXT \
*(.gnu.warning) \
*(.glue_7) \
- *(.glue_7t)
+ *(.glue_7t) \
+ *(.vfp11_veneer) \
+ *(.v4_bx)

#define ARM_TEXT \
IDMAP_TEXT \
diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
index 57fcbf55f913..11ffa79751da 100644
--- a/arch/arm/kernel/vmlinux-xip.lds.S
+++ b/arch/arm/kernel/vmlinux-xip.lds.S
@@ -150,6 +150,7 @@ SECTIONS
_end = .;

STABS_DEBUG
+ DWARF_DEBUG
ARM_DETAILS
}

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 1d3d3b599635..dc672fe35de3 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -149,6 +149,7 @@ SECTIONS
_end = .;

STABS_DEBUG
+ DWARF_DEBUG
ARM_DETAILS
}

--
2.25.1

2020-07-31 23:19:13

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 25/36] arm/build: Warn on orphan section placement

We don't want to depend on the linker's orphan section placement
heuristics as these can vary between linkers, and may change between
versions. All sections need to be explicitly handled in the linker
script.

Specifically, this would have made a recently fixed bug very obvious:

ld: warning: orphan section `.fixup' from `arch/arm/lib/copy_from_user.o' being placed in section `.fixup'

With all sections handled, enable orphan section warning.

Reviewed-by: Nick Desaulniers <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/Makefile | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 59fde2d598d8..e414e3732b3a 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -16,6 +16,10 @@ LDFLAGS_vmlinux += --be8
KBUILD_LDFLAGS_MODULE += --be8
endif

+# We never want expected sections to be placed heuristically by the
+# linker. All sections should be explicitly named in the linker script.
+LDFLAGS_vmlinux += --orphan-handling=warn
+
ifeq ($(CONFIG_ARM_MODULE_PLTS),y)
KBUILD_LDS_MODULE += $(srctree)/arch/arm/kernel/module.lds
endif
--
2.25.1

2020-07-31 23:19:15

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 26/36] arm/boot: Handle all sections explicitly

In preparation for warning on orphan sections, use common macros for
debug sections, discards, and text stubs. Add discards for unwanted .note,
and .rel sections.

Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/boot/compressed/vmlinux.lds.S | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index 09ac33f52814..b914be3a207b 100644
--- a/arch/arm/boot/compressed/vmlinux.lds.S
+++ b/arch/arm/boot/compressed/vmlinux.lds.S
@@ -2,6 +2,7 @@
/*
* Copyright (C) 2000 Russell King
*/
+#include <asm/vmlinux.lds.h>

#ifdef CONFIG_CPU_ENDIAN_BE8
#define ZIMAGE_MAGIC(x) ( (((x) >> 24) & 0x000000ff) | \
@@ -17,8 +18,11 @@ ENTRY(_start)
SECTIONS
{
/DISCARD/ : {
+ COMMON_DISCARDS
*(.ARM.exidx*)
*(.ARM.extab*)
+ *(.note.*)
+ *(.rel.*)
/*
* Discard any r/w data - this produces a link error if we have any,
* which is required for PIC decompression. Local data generates
@@ -36,9 +40,7 @@ SECTIONS
*(.start)
*(.text)
*(.text.*)
- *(.gnu.warning)
- *(.glue_7t)
- *(.glue_7)
+ ARM_STUBS_TEXT
}
.table : ALIGN(4) {
_table_start = .;
@@ -128,12 +130,10 @@ SECTIONS
PROVIDE(__pecoff_data_size = ALIGN(512) - ADDR(.data));
PROVIDE(__pecoff_end = ALIGN(512));

- .stab 0 : { *(.stab) }
- .stabstr 0 : { *(.stabstr) }
- .stab.excl 0 : { *(.stab.excl) }
- .stab.exclstr 0 : { *(.stab.exclstr) }
- .stab.index 0 : { *(.stab.index) }
- .stab.indexstr 0 : { *(.stab.indexstr) }
- .comment 0 : { *(.comment) }
+ STABS_DEBUG
+ DWARF_DEBUG
+ ARM_DETAILS
+
+ ARM_ASSERTS
}
ASSERT(_edata_real == _edata, "error: zImage file size is incorrect");
--
2.25.1

2020-07-31 23:20:01

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 27/36] arm/boot: Warn on orphan section placement

We don't want to depend on the linker's orphan section placement
heuristics as these can vary between linkers, and may change between
versions. All sections need to be explicitly handled in the linker script.

With all sections now handled, enable orphan section warning.

Signed-off-by: Kees Cook <[email protected]>
---
arch/arm/boot/compressed/Makefile | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 00602a6fba04..b8a97d81662d 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -128,6 +128,8 @@ endif
LDFLAGS_vmlinux += --no-undefined
# Delete all temporary local symbols
LDFLAGS_vmlinux += -X
+# Report orphan sections
+LDFLAGS_vmlinux += --orphan-handling=warn
# Next argument is a linker script
LDFLAGS_vmlinux += -T

--
2.25.1

2020-07-31 23:20:22

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 18/36] arm64/build: Use common DISCARDS in linker script

Use the common DISCARDS rule for the linker script in an effort to
regularize the linker script to prepare for warning on orphaned
sections. Additionally clean up left-over no-op macros.

Signed-off-by: Kees Cook <[email protected]>
Acked-by: Will Deacon <[email protected]>
---
arch/arm64/kernel/vmlinux.lds.S | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index b29081d16a70..5c1960406b08 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -6,6 +6,7 @@
*/

#define RO_EXCEPTION_TABLE_ALIGN 8
+#define RUNTIME_DISCARD_EXIT

#include <asm-generic/vmlinux.lds.h>
#include <asm/cache.h>
@@ -89,10 +90,8 @@ SECTIONS
* matching the same input section name. There is no documented
* order of matching.
*/
+ DISCARDS
/DISCARD/ : {
- EXIT_CALL
- *(.discard)
- *(.discard.*)
*(.interp .dynamic)
*(.dynsym .dynstr .hash .gnu.hash)
}
--
2.25.1

2020-07-31 23:21:28

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 29/36] x86/build: Enforce an empty .got.plt section

The .got.plt section should always be zero (or filled only with the
linker-generated lazy dispatch entry). Enforce this with an assert and
mark the section as NOLOAD. This is more sensitive than just blindly
discarding the section.

Signed-off-by: Kees Cook <[email protected]>
---
arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 0cc035cb15f1..7faffe7414d6 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -414,8 +414,20 @@ SECTIONS
ELF_DETAILS

DISCARDS
-}

+ /*
+ * Make sure that the .got.plt is either completely empty or it
+ * contains only the lazy dispatch entries.
+ */
+ .got.plt (NOLOAD) : { *(.got.plt) }
+ ASSERT(SIZEOF(.got.plt) == 0 ||
+#ifdef CONFIG_X86_64
+ SIZEOF(.got.plt) == 0x18,
+#else
+ SIZEOF(.got.plt) == 0xc,
+#endif
+ "Unexpected GOT/PLT entries detected!")
+}

#ifdef CONFIG_X86_32
/*
--
2.25.1

2020-07-31 23:21:37

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 28/36] x86/asm: Avoid generating unused kprobe sections

When !CONFIG_KPROBES, do not generate kprobe sections. This makes
sure there are no unexpected sections encountered by the linker scripts.

Signed-off-by: Kees Cook <[email protected]>
---
arch/x86/include/asm/asm.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 0f63585edf5f..92feec0f0a12 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -138,11 +138,15 @@
# define _ASM_EXTABLE_FAULT(from, to) \
_ASM_EXTABLE_HANDLE(from, to, ex_handler_fault)

-# define _ASM_NOKPROBE(entry) \
+# ifdef CONFIG_KPROBES
+# define _ASM_NOKPROBE(entry) \
.pushsection "_kprobe_blacklist","aw" ; \
_ASM_ALIGN ; \
_ASM_PTR (entry); \
.popsection
+# else
+# define _ASM_NOKPROBE(entry)
+# endif

#else
# define _EXPAND_EXTABLE_HANDLE(x) #x
--
2.25.1

2020-07-31 23:22:03

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 17/36] arm64/build: Remove .eh_frame* sections due to unwind tables

Avoid .eh_frame* section generation by making sure both CFLAGS and AFLAGS
contain -fno-asychronous-unwind-tables and -fno-unwind-tables.

With all sources of .eh_frame now removed from the build, drop this
DISCARD so we can be alerted in the future if it returns unexpectedly
once orphan section warnings have been enabled.

Suggested-by: Ard Biesheuvel <[email protected]>
Acked-by: Will Deacon <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
arch/arm64/Makefile | 5 ++++-
arch/arm64/kernel/vmlinux.lds.S | 1 -
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 70f5905954dd..35de43c29873 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -47,13 +47,16 @@ endif

KBUILD_CFLAGS += -mgeneral-regs-only \
$(compat_vdso) $(cc_has_k_constraint)
-KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
KBUILD_CFLAGS += $(call cc-disable-warning, psabi)
KBUILD_AFLAGS += $(compat_vdso)

KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)

+# Avoid generating .eh_frame* sections.
+KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
+KBUILD_AFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
+
ifeq ($(CONFIG_STACKPROTECTOR_PER_TASK),y)
prepare: stack_protector_prepare
stack_protector_prepare: prepare0
diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index df2916b25ee0..b29081d16a70 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -95,7 +95,6 @@ SECTIONS
*(.discard.*)
*(.interp .dynamic)
*(.dynsym .dynstr .hash .gnu.hash)
- *(.eh_frame)
}

. = KIMAGE_VADDR + TEXT_OFFSET;
--
2.25.1

2020-07-31 23:22:43

by Kees Cook

[permalink] [raw]
Subject: [PATCH v5 19/36] arm64/build: Add missing DWARF sections

Explicitly include DWARF sections when they're present in the build.

Signed-off-by: Kees Cook <[email protected]>
---
arch/arm64/kernel/vmlinux.lds.S | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
index 5c1960406b08..4cf825301c3a 100644
--- a/arch/arm64/kernel/vmlinux.lds.S
+++ b/arch/arm64/kernel/vmlinux.lds.S
@@ -240,6 +240,7 @@ SECTIONS
_end = .;

STABS_DEBUG
+ DWARF_DEBUG
ELF_DETAILS

HEAD_SYMBOLS
--
2.25.1

2020-08-01 02:15:11

by Arvind Sankar

[permalink] [raw]
Subject: Re: [PATCH v5 29/36] x86/build: Enforce an empty .got.plt section

On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote:
> The .got.plt section should always be zero (or filled only with the
> linker-generated lazy dispatch entry). Enforce this with an assert and
> mark the section as NOLOAD. This is more sensitive than just blindly
> discarding the section.
>
> Signed-off-by: Kees Cook <[email protected]>
> ---
> arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 0cc035cb15f1..7faffe7414d6 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -414,8 +414,20 @@ SECTIONS
> ELF_DETAILS
>
> DISCARDS
> -}
>
> + /*
> + * Make sure that the .got.plt is either completely empty or it
> + * contains only the lazy dispatch entries.
> + */
> + .got.plt (NOLOAD) : { *(.got.plt) }
> + ASSERT(SIZEOF(.got.plt) == 0 ||
> +#ifdef CONFIG_X86_64
> + SIZEOF(.got.plt) == 0x18,
> +#else
> + SIZEOF(.got.plt) == 0xc,
> +#endif
> + "Unexpected GOT/PLT entries detected!")
> +}
>
> #ifdef CONFIG_X86_32
> /*
> --
> 2.25.1
>

Is this actually needed? vmlinux is a position-dependent executable, and
it doesn't get linked with any shared libraries, so it should never have
a .got or .got.plt at all I think? Does it show up as an orphan without
this?

2020-08-01 05:35:54

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v5 29/36] x86/build: Enforce an empty .got.plt section

On Fri, Jul 31, 2020 at 10:12:48PM -0400, Arvind Sankar wrote:
> On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote:
> > The .got.plt section should always be zero (or filled only with the
> > linker-generated lazy dispatch entry). Enforce this with an assert and
> > mark the section as NOLOAD. This is more sensitive than just blindly
> > discarding the section.
> >
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index 0cc035cb15f1..7faffe7414d6 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -414,8 +414,20 @@ SECTIONS
> > ELF_DETAILS
> >
> > DISCARDS
> > -}
> >
> > + /*
> > + * Make sure that the .got.plt is either completely empty or it
> > + * contains only the lazy dispatch entries.
> > + */
> > + .got.plt (NOLOAD) : { *(.got.plt) }
> > + ASSERT(SIZEOF(.got.plt) == 0 ||
> > +#ifdef CONFIG_X86_64
> > + SIZEOF(.got.plt) == 0x18,
> > +#else
> > + SIZEOF(.got.plt) == 0xc,
> > +#endif
> > + "Unexpected GOT/PLT entries detected!")
> > +}
> >
> > #ifdef CONFIG_X86_32
> > /*
> > --
> > 2.25.1
> >
>
> Is this actually needed? vmlinux is a position-dependent executable, and
> it doesn't get linked with any shared libraries, so it should never have
> a .got or .got.plt at all I think? Does it show up as an orphan without
> this?

When I switched from DISCARD to 0-assert, I tested all of these, but given
so many combinations, perhaps I made a mistake. I will double-check and
report back.

--
Kees Cook

2020-08-03 19:02:49

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH v5 23/36] arm/build: Explicitly keep .ARM.attributes sections

On Fri, Jul 31, 2020 at 4:18 PM Kees Cook <[email protected]> wrote:
>
> In preparation for adding --orphan-handling=warn, explicitly keep the
> .ARM.attributes section by expanding the existing ELF_DETAILS macro into
> ARM_DETAILS.
>
> Suggested-by: Nick Desaulniers <[email protected]>
> Link: https://lore.kernel.org/lkml/CAKwvOdk-racgq5pxsoGS6Vtifbtrk5fmkmnoLxrQMaOvV0nPWw@mail.gmail.com/
> Signed-off-by: Kees Cook <[email protected]>
> ---
> arch/arm/include/asm/vmlinux.lds.h | 4 ++++
> arch/arm/kernel/vmlinux-xip.lds.S | 2 +-
> arch/arm/kernel/vmlinux.lds.S | 2 +-
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
> index a08f4301b718..c4af5182ab48 100644
> --- a/arch/arm/include/asm/vmlinux.lds.h
> +++ b/arch/arm/include/asm/vmlinux.lds.h
> @@ -52,6 +52,10 @@
> ARM_MMU_DISCARD(*(__ex_table)) \
> COMMON_DISCARDS
>
> +#define ARM_DETAILS \
> + ELF_DETAILS \
> + .ARM.attributes 0 : { *(.ARM.attributes) }

I had to look up what the `0` meant:
https://sourceware.org/binutils/docs/ld/Output-Section-Attributes.html#Output-Section-Attributes
mentions it's an "address" and
https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_chapter/ld_3.html#SEC21
mentions it as "start" (an address).
Unless we need those, can we drop them? (Sorry for the resulting churn
that would cause). I think the NO_LOAD stuff makes more sense, but
I'm curious if the kernel checks for that.

> +
> #define ARM_STUBS_TEXT \
> *(.gnu.warning) \
> *(.glue_7) \
> diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
> index 904c31fa20ed..57fcbf55f913 100644
> --- a/arch/arm/kernel/vmlinux-xip.lds.S
> +++ b/arch/arm/kernel/vmlinux-xip.lds.S
> @@ -150,7 +150,7 @@ SECTIONS
> _end = .;
>
> STABS_DEBUG
> - ELF_DETAILS
> + ARM_DETAILS
> }
>
> /*
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index bb950c896a67..1d3d3b599635 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -149,7 +149,7 @@ SECTIONS
> _end = .;
>
> STABS_DEBUG
> - ELF_DETAILS
> + ARM_DETAILS
> }
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> --
> 2.25.1
>


--
Thanks,
~Nick Desaulniers

2020-08-17 23:14:57

by Fangrui Song

[permalink] [raw]
Subject: Re: [PATCH v5 23/36] arm/build: Explicitly keep .ARM.attributes sections

On 2020-08-03, 'Nick Desaulniers' via Clang Built Linux wrote:
>On Fri, Jul 31, 2020 at 4:18 PM Kees Cook <[email protected]> wrote:
>>
>> In preparation for adding --orphan-handling=warn, explicitly keep the
>> .ARM.attributes section by expanding the existing ELF_DETAILS macro into
>> ARM_DETAILS.
>>
>> Suggested-by: Nick Desaulniers <[email protected]>
>> Link: https://lore.kernel.org/lkml/CAKwvOdk-racgq5pxsoGS6Vtifbtrk5fmkmnoLxrQMaOvV0nPWw@mail.gmail.com/
>> Signed-off-by: Kees Cook <[email protected]>
>> ---
>> arch/arm/include/asm/vmlinux.lds.h | 4 ++++
>> arch/arm/kernel/vmlinux-xip.lds.S | 2 +-
>> arch/arm/kernel/vmlinux.lds.S | 2 +-
>> 3 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
>> index a08f4301b718..c4af5182ab48 100644
>> --- a/arch/arm/include/asm/vmlinux.lds.h
>> +++ b/arch/arm/include/asm/vmlinux.lds.h
>> @@ -52,6 +52,10 @@
>> ARM_MMU_DISCARD(*(__ex_table)) \
>> COMMON_DISCARDS
>>
>> +#define ARM_DETAILS \
>> + ELF_DETAILS \
>> + .ARM.attributes 0 : { *(.ARM.attributes) }
>
>I had to look up what the `0` meant:
>https://sourceware.org/binutils/docs/ld/Output-Section-Attributes.html#Output-Section-Attributes
>mentions it's an "address" and
>https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_chapter/ld_3.html#SEC21
>mentions it as "start" (an address).
>Unless we need those, can we drop them? (Sorry for the resulting churn
>that would cause). I think the NO_LOAD stuff makes more sense, but
>I'm curious if the kernel checks for that.

NOLOAD means SHT_NOBITS (usually SHF_ALLOC). .ARM.attributes is a
non-SHF_ALLOC section.

An explicit 0 (output section address) is good - GNU ld's internal
linker scripts (ld --verbose output) use 0 for such non-SHF_ALLOC sections.
Without the 0, the section may get a non-zero address, which is not
wrong - but probably does not look well. See https://reviews.llvm.org/D85867 for details.


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

>> +
>> #define ARM_STUBS_TEXT \
>> *(.gnu.warning) \
>> *(.glue_7) \
>> diff --git a/arch/arm/kernel/vmlinux-xip.lds.S b/arch/arm/kernel/vmlinux-xip.lds.S
>> index 904c31fa20ed..57fcbf55f913 100644
>> --- a/arch/arm/kernel/vmlinux-xip.lds.S
>> +++ b/arch/arm/kernel/vmlinux-xip.lds.S
>> @@ -150,7 +150,7 @@ SECTIONS
>> _end = .;
>>
>> STABS_DEBUG
>> - ELF_DETAILS
>> + ARM_DETAILS
>> }
>>
>> /*
>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
>> index bb950c896a67..1d3d3b599635 100644
>> --- a/arch/arm/kernel/vmlinux.lds.S
>> +++ b/arch/arm/kernel/vmlinux.lds.S
>> @@ -149,7 +149,7 @@ SECTIONS
>> _end = .;
>>
>> STABS_DEBUG
>> - ELF_DETAILS
>> + ARM_DETAILS
>> }
>>
>> #ifdef CONFIG_STRICT_KERNEL_RWX
>> --
>> 2.25.1
>>

2020-08-21 17:50:12

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v5 29/36] x86/build: Enforce an empty .got.plt section

On Fri, Jul 31, 2020 at 10:12:48PM -0400, Arvind Sankar wrote:
> On Fri, Jul 31, 2020 at 04:08:13PM -0700, Kees Cook wrote:
> > The .got.plt section should always be zero (or filled only with the
> > linker-generated lazy dispatch entry). Enforce this with an assert and
> > mark the section as NOLOAD. This is more sensitive than just blindly
> > discarding the section.
> >
> > Signed-off-by: Kees Cook <[email protected]>
> > ---
> > arch/x86/kernel/vmlinux.lds.S | 14 +++++++++++++-
> > 1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> > index 0cc035cb15f1..7faffe7414d6 100644
> > --- a/arch/x86/kernel/vmlinux.lds.S
> > +++ b/arch/x86/kernel/vmlinux.lds.S
> > @@ -414,8 +414,20 @@ SECTIONS
> > ELF_DETAILS
> >
> > DISCARDS
> > -}
> >
> > + /*
> > + * Make sure that the .got.plt is either completely empty or it
> > + * contains only the lazy dispatch entries.
> > + */
> > + .got.plt (NOLOAD) : { *(.got.plt) }
> > + ASSERT(SIZEOF(.got.plt) == 0 ||
> > +#ifdef CONFIG_X86_64
> > + SIZEOF(.got.plt) == 0x18,
> > +#else
> > + SIZEOF(.got.plt) == 0xc,
> > +#endif
> > + "Unexpected GOT/PLT entries detected!")
> > +}
> >
> > #ifdef CONFIG_X86_32
> > /*
> > --
> > 2.25.1
> >
>
> Is this actually needed? vmlinux is a position-dependent executable, and
> it doesn't get linked with any shared libraries, so it should never have
> a .got or .got.plt at all I think? Does it show up as an orphan without
> this?

Yup, I see this:

/usr/bin/ld.bfd: warning: orphan section `.got.plt' from `arch/x86/kernel/head_64.o' being placed in section `.got.plt'


--
Kees Cook