|
| 1 | +From f43cc4ed67f44f22754bef5bec98bf8aad3e6e8d Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kees Cook < [email protected]> |
| 3 | +Date: Tue, 13 Aug 2019 16:04:50 -0700 |
| 4 | +Subject: [PATCH] arm64/efi: Move variable assignments after SECTIONS |
| 5 | + |
| 6 | +It seems that LLVM's linker does not correctly handle variable assignments |
| 7 | +involving section positions that are updated during the SECTIONS |
| 8 | +parsing. Commit aa69fb62bea1 ("arm64/efi: Mark __efistub_stext_offset as |
| 9 | +an absolute symbol explicitly") ran into this too, but found a different |
| 10 | +workaround. |
| 11 | + |
| 12 | +However, this was not enough, as other variables were also miscalculated |
| 13 | +which manifested as boot failures under UEFI where __efistub__end was |
| 14 | +not taking the correct _end value (they should be the same): |
| 15 | + |
| 16 | +$ ld.lld -EL -maarch64elf --no-undefined -X -shared \ |
| 17 | + -Bsymbolic -z notext -z norelro --no-apply-dynamic-relocs \ |
| 18 | + -o vmlinux.lld -T poc.lds --whole-archive vmlinux.o && \ |
| 19 | + readelf -Ws vmlinux.lld | egrep '\b(__efistub_|)_end\b' |
| 20 | +368272: ffff000002218000 0 NOTYPE LOCAL HIDDEN 38 __efistub__end |
| 21 | +368322: ffff000012318000 0 NOTYPE GLOBAL DEFAULT 38 _end |
| 22 | + |
| 23 | +$ aarch64-linux-gnu-ld.bfd -EL -maarch64elf --no-undefined -X -shared \ |
| 24 | + -Bsymbolic -z notext -z norelro --no-apply-dynamic-relocs \ |
| 25 | + -o vmlinux.bfd -T poc.lds --whole-archive vmlinux.o && \ |
| 26 | + readelf -Ws vmlinux.bfd | egrep '\b(__efistub_|)_end\b' |
| 27 | +338124: ffff000012318000 0 NOTYPE LOCAL DEFAULT ABS __efistub__end |
| 28 | +383812: ffff000012318000 0 NOTYPE GLOBAL DEFAULT 15325 _end |
| 29 | + |
| 30 | +To work around this, all of the __efistub_-prefixed variable assignments |
| 31 | +need to be moved after the linker script's SECTIONS entry. As it turns |
| 32 | +out, this also solves the problem fixed in commit aa69fb62bea1, so those |
| 33 | +changes are reverted here. |
| 34 | + |
| 35 | +Link: https://github.com/ClangBuiltLinux/linux/issues/634 |
| 36 | +Link: https://bugs.llvm.org/show_bug.cgi?id=42990 |
| 37 | +Acked-by: Ard Biesheuvel < [email protected]> |
| 38 | +Signed-off-by: Kees Cook < [email protected]> |
| 39 | +Signed-off-by: Will Deacon < [email protected]> |
| 40 | +(am from https://git.kernel.org/arm64/c/90776dd1c427cbb4d381aa4b13338f1fb1d20f5e) |
| 41 | +Signed-off-by: Nathan Chancellor < [email protected]> |
| 42 | +--- |
| 43 | + arch/arm64/kernel/image-vars.h | 51 +++++++++++++++++++++++++++++++++ |
| 44 | + arch/arm64/kernel/image.h | 42 --------------------------- |
| 45 | + arch/arm64/kernel/vmlinux.lds.S | 2 ++ |
| 46 | + 3 files changed, 53 insertions(+), 42 deletions(-) |
| 47 | + create mode 100644 arch/arm64/kernel/image-vars.h |
| 48 | + |
| 49 | +diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h |
| 50 | +new file mode 100644 |
| 51 | +index 000000000000..25a2a9b479c2 |
| 52 | +--- /dev/null |
| 53 | ++++ b/arch/arm64/kernel/image-vars.h |
| 54 | +@@ -0,0 +1,51 @@ |
| 55 | ++/* SPDX-License-Identifier: GPL-2.0-only */ |
| 56 | ++/* |
| 57 | ++ * Linker script variables to be set after section resolution, as |
| 58 | ++ * ld.lld does not like variables assigned before SECTIONS is processed. |
| 59 | ++ */ |
| 60 | ++#ifndef __ARM64_KERNEL_IMAGE_VARS_H |
| 61 | ++#define __ARM64_KERNEL_IMAGE_VARS_H |
| 62 | ++ |
| 63 | ++#ifndef LINKER_SCRIPT |
| 64 | ++#error This file should only be included in vmlinux.lds.S |
| 65 | ++#endif |
| 66 | ++ |
| 67 | ++#ifdef CONFIG_EFI |
| 68 | ++ |
| 69 | ++__efistub_stext_offset = stext - _text; |
| 70 | ++ |
| 71 | ++/* |
| 72 | ++ * The EFI stub has its own symbol namespace prefixed by __efistub_, to |
| 73 | ++ * isolate it from the kernel proper. The following symbols are legally |
| 74 | ++ * accessed by the stub, so provide some aliases to make them accessible. |
| 75 | ++ * Only include data symbols here, or text symbols of functions that are |
| 76 | ++ * guaranteed to be safe when executed at another offset than they were |
| 77 | ++ * linked at. The routines below are all implemented in assembler in a |
| 78 | ++ * position independent manner |
| 79 | ++ */ |
| 80 | ++__efistub_memcmp = __pi_memcmp; |
| 81 | ++__efistub_memchr = __pi_memchr; |
| 82 | ++__efistub_memcpy = __pi_memcpy; |
| 83 | ++__efistub_memmove = __pi_memmove; |
| 84 | ++__efistub_memset = __pi_memset; |
| 85 | ++__efistub_strlen = __pi_strlen; |
| 86 | ++__efistub_strnlen = __pi_strnlen; |
| 87 | ++__efistub_strcmp = __pi_strcmp; |
| 88 | ++__efistub_strncmp = __pi_strncmp; |
| 89 | ++__efistub_strrchr = __pi_strrchr; |
| 90 | ++__efistub___flush_dcache_area = __pi___flush_dcache_area; |
| 91 | ++ |
| 92 | ++#ifdef CONFIG_KASAN |
| 93 | ++__efistub___memcpy = __pi_memcpy; |
| 94 | ++__efistub___memmove = __pi_memmove; |
| 95 | ++__efistub___memset = __pi_memset; |
| 96 | ++#endif |
| 97 | ++ |
| 98 | ++__efistub__text = _text; |
| 99 | ++__efistub__end = _end; |
| 100 | ++__efistub__edata = _edata; |
| 101 | ++__efistub_screen_info = screen_info; |
| 102 | ++ |
| 103 | ++#endif |
| 104 | ++ |
| 105 | ++#endif /* __ARM64_KERNEL_IMAGE_VARS_H */ |
| 106 | +diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h |
| 107 | +index 2b85c0d6fa3d..c7d38c660372 100644 |
| 108 | +--- a/arch/arm64/kernel/image.h |
| 109 | ++++ b/arch/arm64/kernel/image.h |
| 110 | +@@ -65,46 +65,4 @@ |
| 111 | + DEFINE_IMAGE_LE64(_kernel_offset_le, TEXT_OFFSET); \ |
| 112 | + DEFINE_IMAGE_LE64(_kernel_flags_le, __HEAD_FLAGS); |
| 113 | + |
| 114 | +-#ifdef CONFIG_EFI |
| 115 | +- |
| 116 | +-/* |
| 117 | +- * Use ABSOLUTE() to avoid ld.lld treating this as a relative symbol: |
| 118 | +- * https://github.com/ClangBuiltLinux/linux/issues/561 |
| 119 | +- */ |
| 120 | +-__efistub_stext_offset = ABSOLUTE(stext - _text); |
| 121 | +- |
| 122 | +-/* |
| 123 | +- * The EFI stub has its own symbol namespace prefixed by __efistub_, to |
| 124 | +- * isolate it from the kernel proper. The following symbols are legally |
| 125 | +- * accessed by the stub, so provide some aliases to make them accessible. |
| 126 | +- * Only include data symbols here, or text symbols of functions that are |
| 127 | +- * guaranteed to be safe when executed at another offset than they were |
| 128 | +- * linked at. The routines below are all implemented in assembler in a |
| 129 | +- * position independent manner |
| 130 | +- */ |
| 131 | +-__efistub_memcmp = __pi_memcmp; |
| 132 | +-__efistub_memchr = __pi_memchr; |
| 133 | +-__efistub_memcpy = __pi_memcpy; |
| 134 | +-__efistub_memmove = __pi_memmove; |
| 135 | +-__efistub_memset = __pi_memset; |
| 136 | +-__efistub_strlen = __pi_strlen; |
| 137 | +-__efistub_strnlen = __pi_strnlen; |
| 138 | +-__efistub_strcmp = __pi_strcmp; |
| 139 | +-__efistub_strncmp = __pi_strncmp; |
| 140 | +-__efistub_strrchr = __pi_strrchr; |
| 141 | +-__efistub___flush_dcache_area = __pi___flush_dcache_area; |
| 142 | +- |
| 143 | +-#ifdef CONFIG_KASAN |
| 144 | +-__efistub___memcpy = __pi_memcpy; |
| 145 | +-__efistub___memmove = __pi_memmove; |
| 146 | +-__efistub___memset = __pi_memset; |
| 147 | +-#endif |
| 148 | +- |
| 149 | +-__efistub__text = _text; |
| 150 | +-__efistub__end = _end; |
| 151 | +-__efistub__edata = _edata; |
| 152 | +-__efistub_screen_info = screen_info; |
| 153 | +- |
| 154 | +-#endif |
| 155 | +- |
| 156 | + #endif /* __ARM64_KERNEL_IMAGE_H */ |
| 157 | +diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S |
| 158 | +index 7fa008374907..803b24d2464a 100644 |
| 159 | +--- a/arch/arm64/kernel/vmlinux.lds.S |
| 160 | ++++ b/arch/arm64/kernel/vmlinux.lds.S |
| 161 | +@@ -245,6 +245,8 @@ SECTIONS |
| 162 | + HEAD_SYMBOLS |
| 163 | + } |
| 164 | + |
| 165 | ++#include "image-vars.h" |
| 166 | ++ |
| 167 | + /* |
| 168 | + * The HYP init code and ID map text can't be longer than a page each, |
| 169 | + * and should not cross a page boundary. |
| 170 | +-- |
| 171 | +2.23.0.rc2 |
| 172 | + |
0 commit comments