* [PATCH v2 0/2] arm: Warn on orphan section placement
@ 2020-06-22 20:49 Kees Cook
2020-06-22 20:49 ` [PATCH v2 1/2] arm/build: " Kees Cook
2020-06-22 20:49 ` [PATCH v2 2/2] arm/boot: " Kees Cook
0 siblings, 2 replies; 8+ messages in thread
From: Kees Cook @ 2020-06-22 20:49 UTC (permalink / raw)
To: Russell King
Cc: Kees Cook, Masahiro Yamada, Nick Desaulniers, Nathan Chancellor,
Will Deacon, Ard Biesheuvel, Arnd Bergmann, linux-arm-kernel,
linux-kernel
v2:
- split by architecture, rebase to v5.8-rc2
v1: https://lore.kernel.org/lkml/20200228002244.15240-1-keescook@chromium.org/
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. Similarly, the recent FGKASLR series brough up orphan
section handling too[2]. In both 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 or
discarded) with any orphans throwing a warning. The powerpc architecture
actually already does this, so this series extends coverage to arm.
This series needs one additional commit that is not yet in
any tree, but I hope to have it landed via x86 -tip shortly:
https://lore.kernel.org/lkml/20200228002244.15240-3-keescook@chromium.org/
Thanks!
-Kees
[1] https://github.com/ClangBuiltLinux/linux/issues/282
[2] https://lore.kernel.org/lkml/202002242122.AA4D1B8@keescook/
Kees Cook (2):
arm/build: Warn on orphan section placement
arm/boot: Warn on orphan section placement
arch/arm/Makefile | 4 ++++
arch/arm/boot/compressed/Makefile | 2 ++
arch/arm/boot/compressed/vmlinux.lds.S | 17 ++++++--------
.../arm/{kernel => include/asm}/vmlinux.lds.h | 22 ++++++++++++++-----
arch/arm/kernel/vmlinux-xip.lds.S | 5 ++---
arch/arm/kernel/vmlinux.lds.S | 5 ++---
6 files changed, 34 insertions(+), 21 deletions(-)
rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (92%)
--
2.25.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/2] arm/build: Warn on orphan section placement
2020-06-22 20:49 [PATCH v2 0/2] arm: Warn on orphan section placement Kees Cook
@ 2020-06-22 20:49 ` Kees Cook
2020-06-24 0:03 ` Nick Desaulniers
2020-06-22 20:49 ` [PATCH v2 2/2] arm/boot: " Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Kees Cook @ 2020-06-22 20:49 UTC (permalink / raw)
To: Russell King
Cc: Kees Cook, Masahiro Yamada, Nick Desaulniers, Nathan Chancellor,
Will Deacon, Ard Biesheuvel, Arnd Bergmann, linux-arm-kernel,
linux-kernel
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 named 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'
Refactor linker script include file for use in standard and XIP linker
scripts, as well as in the coming boot linker script changes. Add debug
sections explicitly. Create ARM_COMMON_DISCARD macro with unneeded
sections .ARM.attributes, .iplt, .rel.iplt, .igot.plt, and .modinfo.
Create ARM_STUBS_TEXT macro with missed text stub sections .vfp11_veneer,
and .v4_bx. Finally enable orphan section warning.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm/Makefile | 4 ++++
.../arm/{kernel => include/asm}/vmlinux.lds.h | 22 ++++++++++++++-----
arch/arm/kernel/vmlinux-xip.lds.S | 5 ++---
arch/arm/kernel/vmlinux.lds.S | 5 ++---
4 files changed, 25 insertions(+), 11 deletions(-)
rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (92%)
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
diff --git a/arch/arm/kernel/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
similarity index 92%
rename from arch/arm/kernel/vmlinux.lds.h
rename to arch/arm/include/asm/vmlinux.lds.h
index 381a8e105fa5..3d88ea74f4cd 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)
@@ -37,6 +38,13 @@
*(.idmap.text) \
__idmap_text_end = .; \
+#define ARM_COMMON_DISCARD \
+ *(.ARM.attributes) \
+ *(.iplt) *(.rel.iplt) *(.igot.plt) \
+ *(.modinfo) \
+ *(.discard) \
+ *(.discard.*)
+
#define ARM_DISCARD \
*(.ARM.exidx.exit.text) \
*(.ARM.extab.exit.text) \
@@ -49,8 +57,14 @@
EXIT_CALL \
ARM_MMU_DISCARD(*(.text.fixup)) \
ARM_MMU_DISCARD(*(__ex_table)) \
- *(.discard) \
- *(.discard.*)
+ ARM_COMMON_DISCARD
+
+#define ARM_STUBS_TEXT \
+ *(.gnu.warning) \
+ *(.glue_7t) \
+ *(.glue_7) \
+ *(.vfp11_veneer) \
+ *(.v4_bx)
#define ARM_TEXT \
IDMAP_TEXT \
@@ -64,9 +78,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 6d2be994ae58..0807f40844a2 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)
@@ -152,6 +150,7 @@ SECTIONS
_end = .;
STABS_DEBUG
+ DWARF_DEBUG
}
/*
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 7f24bc08403e..969205f125ca 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)
@@ -151,6 +149,7 @@ SECTIONS
_end = .;
STABS_DEBUG
+ DWARF_DEBUG
}
#ifdef CONFIG_STRICT_KERNEL_RWX
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/2] arm/boot: Warn on orphan section placement
2020-06-22 20:49 [PATCH v2 0/2] arm: Warn on orphan section placement Kees Cook
2020-06-22 20:49 ` [PATCH v2 1/2] arm/build: " Kees Cook
@ 2020-06-22 20:49 ` Kees Cook
2020-06-24 0:08 ` Nick Desaulniers
1 sibling, 1 reply; 8+ messages in thread
From: Kees Cook @ 2020-06-22 20:49 UTC (permalink / raw)
To: Russell King
Cc: Kees Cook, Masahiro Yamada, Nick Desaulniers, Nathan Chancellor,
Will Deacon, Ard Biesheuvel, Arnd Bergmann, linux-arm-kernel,
linux-kernel
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 named in the linker
script.
Use common macros for debug sections, discards, and text stubs. Add
discards for unwanted .note, and .rel sections. Finally, enable orphan
section warning.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/arm/boot/compressed/Makefile | 2 ++
arch/arm/boot/compressed/vmlinux.lds.S | 17 +++++++----------
2 files changed, 9 insertions(+), 10 deletions(-)
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
diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
index 09ac33f52814..c2a8509f876f 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/ : {
+ ARM_COMMON_DISCARD
*(.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,7 @@ 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
}
ASSERT(_edata_real == _edata, "error: zImage file size is incorrect");
--
2.25.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement
2020-06-22 20:49 ` [PATCH v2 1/2] arm/build: " Kees Cook
@ 2020-06-24 0:03 ` Nick Desaulniers
2020-06-24 19:43 ` Kees Cook
2020-06-26 21:36 ` Nick Desaulniers
0 siblings, 2 replies; 8+ messages in thread
From: Nick Desaulniers @ 2020-06-24 0:03 UTC (permalink / raw)
To: Kees Cook
Cc: Russell King, Masahiro Yamada, Nathan Chancellor, Will Deacon,
Ard Biesheuvel, Arnd Bergmann, Linux ARM, LKML, Eli Friedman
On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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 named 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'
>
> Refactor linker script include file for use in standard and XIP linker
> scripts, as well as in the coming boot linker script changes. Add debug
> sections explicitly. Create ARM_COMMON_DISCARD macro with unneeded
> sections .ARM.attributes, .iplt, .rel.iplt, .igot.plt, and .modinfo.
> Create ARM_STUBS_TEXT macro with missed text stub sections .vfp11_veneer,
> and .v4_bx. Finally enable orphan section warning.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm/Makefile | 4 ++++
> .../arm/{kernel => include/asm}/vmlinux.lds.h | 22 ++++++++++++++-----
> arch/arm/kernel/vmlinux-xip.lds.S | 5 ++---
> arch/arm/kernel/vmlinux.lds.S | 5 ++---
> 4 files changed, 25 insertions(+), 11 deletions(-)
> rename arch/arm/{kernel => include/asm}/vmlinux.lds.h (92%)
>
> 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
> diff --git a/arch/arm/kernel/vmlinux.lds.h b/arch/arm/include/asm/vmlinux.lds.h
> similarity index 92%
> rename from arch/arm/kernel/vmlinux.lds.h
> rename to arch/arm/include/asm/vmlinux.lds.h
> index 381a8e105fa5..3d88ea74f4cd 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)
> @@ -37,6 +38,13 @@
> *(.idmap.text) \
> __idmap_text_end = .; \
>
> +#define ARM_COMMON_DISCARD \
> + *(.ARM.attributes) \
I could have sworn that someone (Eli?) once told me that this section
(.ARM.attributes) is used for disambiguating which ARM version or
which optional extensions were used when compiling, and that without
this section, one would not be able to disassemble 32b ARM precisely.
If that's the case, we might not want to discard it?
In fact, in LLVM, I can see quite a few tests under
llvm/test/MC/ARM/directive-arch-armv*.s that reference
.ARM.attributes. Looks like `{llvm|arm-linux-gnueabihf}-readelf
--arch-specific` can be used to dump these sections. Though I also
only see code in LLVM's tree for writing this, not necessarily reading
it. Only did a cursory scan of
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp.
Otherwise patch LGTM.
> + *(.iplt) *(.rel.iplt) *(.igot.plt) \
> + *(.modinfo) \
> + *(.discard) \
> + *(.discard.*)
> +
> #define ARM_DISCARD \
> *(.ARM.exidx.exit.text) \
> *(.ARM.extab.exit.text) \
> @@ -49,8 +57,14 @@
> EXIT_CALL \
> ARM_MMU_DISCARD(*(.text.fixup)) \
> ARM_MMU_DISCARD(*(__ex_table)) \
> - *(.discard) \
> - *(.discard.*)
> + ARM_COMMON_DISCARD
> +
> +#define ARM_STUBS_TEXT \
> + *(.gnu.warning) \
> + *(.glue_7t) \
> + *(.glue_7) \
This changes the order of .glue_7t relative to .glue_7. Maybe that
doesn't matter.
> + *(.vfp11_veneer) \
> + *(.v4_bx)
>
> #define ARM_TEXT \
> IDMAP_TEXT \
> @@ -64,9 +78,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 6d2be994ae58..0807f40844a2 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)
>
> @@ -152,6 +150,7 @@ SECTIONS
> _end = .;
>
> STABS_DEBUG
> + DWARF_DEBUG
> }
>
> /*
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 7f24bc08403e..969205f125ca 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)
>
> @@ -151,6 +149,7 @@ SECTIONS
> _end = .;
>
> STABS_DEBUG
> + DWARF_DEBUG
> }
>
> #ifdef CONFIG_STRICT_KERNEL_RWX
> --
> 2.25.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/2] arm/boot: Warn on orphan section placement
2020-06-22 20:49 ` [PATCH v2 2/2] arm/boot: " Kees Cook
@ 2020-06-24 0:08 ` Nick Desaulniers
0 siblings, 0 replies; 8+ messages in thread
From: Nick Desaulniers @ 2020-06-24 0:08 UTC (permalink / raw)
To: Kees Cook
Cc: Russell King, Masahiro Yamada, Nathan Chancellor, Will Deacon,
Ard Biesheuvel, Arnd Bergmann, Linux ARM, LKML
On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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 named in the linker
> script.
>
> Use common macros for debug sections, discards, and text stubs. Add
> discards for unwanted .note, and .rel sections. Finally, enable orphan
> section warning.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> arch/arm/boot/compressed/Makefile | 2 ++
> arch/arm/boot/compressed/vmlinux.lds.S | 17 +++++++----------
> 2 files changed, 9 insertions(+), 10 deletions(-)
>
> 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
>
> diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S
> index 09ac33f52814..c2a8509f876f 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/ : {
> + ARM_COMMON_DISCARD
> *(.ARM.exidx*)
> *(.ARM.extab*)
> + *(.note.*)
> + *(.rel.*)
.rel.* is the only case I'm curious about. Why do we want it in
vmlinux, but not the compressed image? Should `.rel.*` just be part
of ARM_COMMON_DISCARD from the previous patch?
> /*
> * 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,7 @@ 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
> }
> ASSERT(_edata_real == _edata, "error: zImage file size is incorrect");
> --
> 2.25.1
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement
2020-06-24 0:03 ` Nick Desaulniers
@ 2020-06-24 19:43 ` Kees Cook
2020-06-26 21:36 ` Nick Desaulniers
1 sibling, 0 replies; 8+ messages in thread
From: Kees Cook @ 2020-06-24 19:43 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Russell King, Masahiro Yamada, Nathan Chancellor, Will Deacon,
Ard Biesheuvel, Arnd Bergmann, Linux ARM, LKML, Eli Friedman
On Tue, Jun 23, 2020 at 05:03:46PM -0700, Nick Desaulniers wrote:
> On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@chromium.org> wrote:
> > [...]
> > @@ -37,6 +38,13 @@
> > *(.idmap.text) \
> > __idmap_text_end = .; \
> >
> > +#define ARM_COMMON_DISCARD \
> > + *(.ARM.attributes) \
>
> I could have sworn that someone (Eli?) once told me that this section
> (.ARM.attributes) is used for disambiguating which ARM version or
> which optional extensions were used when compiling, and that without
> this section, one would not be able to disassemble 32b ARM precisely.
> If that's the case, we might not want to discard it?
Perhaps we want to treat it like .comment and include it in the ELF?
> > +#define ARM_STUBS_TEXT \
> > + *(.gnu.warning) \
> > + *(.glue_7t) \
> > + *(.glue_7) \
>
> This changes the order of .glue_7t relative to .glue_7. Maybe that
> doesn't matter.
Good point. I'll swap it just for consistency.
Thanks!
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement
2020-06-24 0:03 ` Nick Desaulniers
2020-06-24 19:43 ` Kees Cook
@ 2020-06-26 21:36 ` Nick Desaulniers
2020-06-26 21:55 ` Kees Cook
1 sibling, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2020-06-26 21:36 UTC (permalink / raw)
To: Kees Cook
Cc: Russell King, Masahiro Yamada, Nathan Chancellor, Will Deacon,
Ard Biesheuvel, Arnd Bergmann, Linux ARM, LKML, Eli Friedman
On Tue, Jun 23, 2020 at 5:03 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Mon, Jun 22, 2020 at 1:49 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > --- 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)
> > @@ -37,6 +38,13 @@
> > *(.idmap.text) \
> > __idmap_text_end = .; \
> >
> > +#define ARM_COMMON_DISCARD \
> > + *(.ARM.attributes) \
>
> I could have sworn that someone (Eli?) once told me that this section
> (.ARM.attributes) is used for disambiguating which ARM version or
> which optional extensions were used when compiling, and that without
> this section, one would not be able to disassemble 32b ARM precisely.
> If that's the case, we might not want to discard it?
Yep, looks like ELFObjectFileBase::getARMFeatures() in
llvm/lib/Object/ELFObjectFile.cpp does exactly that and more.
https://github.com/llvm/llvm-project/blob/8808574e7438c8768b78ae7dd0f029385c6df01d/llvm/lib/Object/ELFObjectFile.cpp#L359-L441
https://github.com/llvm/llvm-project/blob/8808574e7438c8768b78ae7dd0f029385c6df01d/llvm/lib/Object/ELFObjectFile.cpp#L159-L287
As a test, let's do:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 -j71 defconfig
(so armv7)
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 -j71
(then pick any random object file)
$ llvm-readelf -S arch/arm/kernel/bugs.o | grep attri
[15] .ARM.attributes ARM_ATTRIBUTES 00000000 0000f7 000037 00 0 0 1
$ llvm-readelf --arch-specific arch/arm/kernel/bugs.o | grep -A 2 CPU_arch
TagName: CPU_arch
Description: ARM v7
}
And let's see if this actually has a difference on the disassembly.
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 -j71
(full build, since we're talking about linker script changes for vmlinux)
$ llvm-objdump -d vmlinux > prepatch.txt
(apply your patch)
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 -j71 clean
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 -j71
$ llvm-objdump -d vmlinux > postpatch.txt
$ diff -u prepatch.txt postpatch.txt | less
No difference. Eh. Checking again with arm-linux-gnueabihf-objdump, it
seems some constants are slightly different for `movw`'s though. Not
sure what's that about.
If I enable CONFIG_THUMB2_KERNEL=y, is where things become
interesting. llvm-objdump produces wildly different disassembly before
vs after removing .ARM.attributes. There's also lots of decode errors
in the disassembly.
Repeating the thumb2 test with GNU objdump, I only see slight
differences in constants values for operands to `movw`. So it looks
like GNU objdump doesn't rely on .ARM.attributes to disambiguate
between ARM vs THUMB2 instructions like llvm-objdump does. We can
probably improve llvm-objdump, but I'd rather not discard this section
for now.
(also, I didn't test armv6, v5, etc, but those might be interesting
tests, too, should we want to discard this section. Also, I think we
can explicitly specify --triple=thumbv7-linux-gnueabihf to
llvm-objdump, but I'd prefer it if my disassembler did the work for
me, since I'm lazy)
(oh man, the bytes are printed with different endianness between
arm-linux-gnueabihf-objdump and llvm-objdump...guessing that's a bug
in llvm).
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 1/2] arm/build: Warn on orphan section placement
2020-06-26 21:36 ` Nick Desaulniers
@ 2020-06-26 21:55 ` Kees Cook
0 siblings, 0 replies; 8+ messages in thread
From: Kees Cook @ 2020-06-26 21:55 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Russell King, Masahiro Yamada, Nathan Chancellor, Will Deacon,
Ard Biesheuvel, Arnd Bergmann, Linux ARM, LKML, Eli Friedman
On Fri, Jun 26, 2020 at 02:36:44PM -0700, Nick Desaulniers wrote:
> If I enable CONFIG_THUMB2_KERNEL=y, is where things become
> interesting. llvm-objdump produces wildly different disassembly before
> vs after removing .ARM.attributes. There's also lots of decode errors
> in the disassembly.
Yeah, at your earlier recommendation my v4 series will be keeping
.ARM.attributes. Thanks for verifying!
--
Kees Cook
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-06-26 21:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 20:49 [PATCH v2 0/2] arm: Warn on orphan section placement Kees Cook
2020-06-22 20:49 ` [PATCH v2 1/2] arm/build: " Kees Cook
2020-06-24 0:03 ` Nick Desaulniers
2020-06-24 19:43 ` Kees Cook
2020-06-26 21:36 ` Nick Desaulniers
2020-06-26 21:55 ` Kees Cook
2020-06-22 20:49 ` [PATCH v2 2/2] arm/boot: " Kees Cook
2020-06-24 0:08 ` Nick Desaulniers
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).