* [PATCH v2 0/2] arm64: Warn on orphan section placement @ 2020-06-22 20:58 Kees Cook 2020-06-22 20:58 ` [PATCH v2 1/2] arm64/build: Use common DISCARDS in linker script Kees Cook 2020-06-22 20:58 ` [PATCH v2 2/2] arm64/build: Warn on orphan section placement Kees Cook 0 siblings, 2 replies; 17+ messages in thread From: Kees Cook @ 2020-06-22 20:58 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Catalin Marinas, Mark Rutland, Ard Biesheuvel, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, 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 arm64. 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/20200622205341.2987797-2-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): arm64/build: Use common DISCARDS in linker script arm64/build: Warn on orphan section placement arch/arm64/Makefile | 4 ++++ arch/arm64/kernel/vmlinux.lds.S | 10 ++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/2] arm64/build: Use common DISCARDS in linker script 2020-06-22 20:58 [PATCH v2 0/2] arm64: Warn on orphan section placement Kees Cook @ 2020-06-22 20:58 ` Kees Cook 2020-06-22 20:58 ` [PATCH v2 2/2] arm64/build: Warn on orphan section placement Kees Cook 1 sibling, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-22 20:58 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Catalin Marinas, Mark Rutland, Ard Biesheuvel, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, linux-arm-kernel, linux-kernel 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 <keescook@chromium.org> Acked-by: Will Deacon <will@kernel.org> --- 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 6827da7f3aa5..5427f502c3a6 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) *(.eh_frame) -- 2.25.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-22 20:58 [PATCH v2 0/2] arm64: Warn on orphan section placement Kees Cook 2020-06-22 20:58 ` [PATCH v2 1/2] arm64/build: Use common DISCARDS in linker script Kees Cook @ 2020-06-22 20:58 ` Kees Cook 2020-06-23 14:52 ` Will Deacon 2020-06-24 3:52 ` kernel test robot 1 sibling, 2 replies; 17+ messages in thread From: Kees Cook @ 2020-06-22 20:58 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Catalin Marinas, Mark Rutland, Ard Biesheuvel, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, 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. Explicitly include debug sections when they're present. Add .eh_frame* to discard as it seems that these are still generated even though -fno-asynchronous-unwind-tables is being specified. Add .plt and .data.rel.ro to discards as they are not actually used. Add .got.plt to the image as it does appear to be mapped near .data. Finally enable orphan section warnings. Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/arm64/Makefile | 4 ++++ arch/arm64/kernel/vmlinux.lds.S | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile index a0d94d063fa8..3e628983445a 100644 --- a/arch/arm64/Makefile +++ b/arch/arm64/Makefile @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 endif 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_ARM64_USE_LSE_ATOMICS), y) ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) $(warning LSE atomics not supported by binutils) diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index 5427f502c3a6..c9ecb3b2007d 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -94,7 +94,8 @@ SECTIONS /DISCARD/ : { *(.interp .dynamic) *(.dynsym .dynstr .hash .gnu.hash) - *(.eh_frame) + *(.plt) *(.data.rel.ro) + *(.eh_frame) *(.init.eh_frame) } . = KIMAGE_VADDR + TEXT_OFFSET; @@ -209,6 +210,7 @@ SECTIONS _data = .; _sdata = .; RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) + .got.plt : ALIGN(8) { *(.got.plt) } /* * Data written with the MMU off but read with the MMU on requires @@ -244,6 +246,7 @@ SECTIONS _end = .; STABS_DEBUG + DWARF_DEBUG HEAD_SYMBOLS } -- 2.25.1 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-22 20:58 ` [PATCH v2 2/2] arm64/build: Warn on orphan section placement Kees Cook @ 2020-06-23 14:52 ` Will Deacon 2020-06-24 3:52 ` kernel test robot 1 sibling, 0 replies; 17+ messages in thread From: Will Deacon @ 2020-06-23 14:52 UTC (permalink / raw) To: Kees Cook Cc: Catalin Marinas, Mark Rutland, Ard Biesheuvel, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, linux-arm-kernel, linux-kernel On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > Explicitly include debug sections when they're present. Add .eh_frame* > to discard as it seems that these are still generated even though > -fno-asynchronous-unwind-tables is being specified. Add .plt and > .data.rel.ro to discards as they are not actually used. Add .got.plt > to the image as it does appear to be mapped near .data. Finally enable > orphan section warnings. Can you elaborate a bit on what .got.plt is being used for, please? I wonder if there's an interaction with an erratum workaround in the linker or something. > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index a0d94d063fa8..3e628983445a 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > endif > 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_ARM64_USE_LSE_ATOMICS), y) > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > $(warning LSE atomics not supported by binutils) > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 5427f502c3a6..c9ecb3b2007d 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -94,7 +94,8 @@ SECTIONS > /DISCARD/ : { > *(.interp .dynamic) > *(.dynsym .dynstr .hash .gnu.hash) > - *(.eh_frame) > + *(.plt) *(.data.rel.ro) > + *(.eh_frame) *(.init.eh_frame) Do we need to include .eh_frame_hdr here too? > } > > . = KIMAGE_VADDR + TEXT_OFFSET; > @@ -209,6 +210,7 @@ SECTIONS > _data = .; > _sdata = .; > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > + .got.plt : ALIGN(8) { *(.got.plt) } > > /* > * Data written with the MMU off but read with the MMU on requires > @@ -244,6 +246,7 @@ SECTIONS > _end = .; > > STABS_DEBUG > + DWARF_DEBUG I know you didn't add it, but do we _really_ care about stabs debug? Who generates that for arm64? Will ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-23 14:52 ` Will Deacon 0 siblings, 0 replies; 17+ messages in thread From: Will Deacon @ 2020-06-23 14:52 UTC (permalink / raw) To: Kees Cook Cc: Mark Rutland, Arnd Bergmann, Catalin Marinas, Nick Desaulniers, linux-kernel, clang-built-linux, James Morse, Nathan Chancellor, Peter Collingbourne, Ard Biesheuvel, linux-arm-kernel On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > Explicitly include debug sections when they're present. Add .eh_frame* > to discard as it seems that these are still generated even though > -fno-asynchronous-unwind-tables is being specified. Add .plt and > .data.rel.ro to discards as they are not actually used. Add .got.plt > to the image as it does appear to be mapped near .data. Finally enable > orphan section warnings. Can you elaborate a bit on what .got.plt is being used for, please? I wonder if there's an interaction with an erratum workaround in the linker or something. > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > index a0d94d063fa8..3e628983445a 100644 > --- a/arch/arm64/Makefile > +++ b/arch/arm64/Makefile > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > endif > 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_ARM64_USE_LSE_ATOMICS), y) > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > $(warning LSE atomics not supported by binutils) > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 5427f502c3a6..c9ecb3b2007d 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -94,7 +94,8 @@ SECTIONS > /DISCARD/ : { > *(.interp .dynamic) > *(.dynsym .dynstr .hash .gnu.hash) > - *(.eh_frame) > + *(.plt) *(.data.rel.ro) > + *(.eh_frame) *(.init.eh_frame) Do we need to include .eh_frame_hdr here too? > } > > . = KIMAGE_VADDR + TEXT_OFFSET; > @@ -209,6 +210,7 @@ SECTIONS > _data = .; > _sdata = .; > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > + .got.plt : ALIGN(8) { *(.got.plt) } > > /* > * Data written with the MMU off but read with the MMU on requires > @@ -244,6 +246,7 @@ SECTIONS > _end = .; > > STABS_DEBUG > + DWARF_DEBUG I know you didn't add it, but do we _really_ care about stabs debug? Who generates that for arm64? Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-23 14:52 ` Will Deacon @ 2020-06-23 14:59 ` Ard Biesheuvel -1 siblings, 0 replies; 17+ messages in thread From: Ard Biesheuvel @ 2020-06-23 14:59 UTC (permalink / raw) To: Will Deacon Cc: Kees Cook, Catalin Marinas, Mark Rutland, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Linux ARM, Linux Kernel Mailing List On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > to discard as it seems that these are still generated even though > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > to the image as it does appear to be mapped near .data. Finally enable > > orphan section warnings. > > Can you elaborate a bit on what .got.plt is being used for, please? I > wonder if there's an interaction with an erratum workaround in the linker > or something. > .got.plt is not used at all, but it has three magic entries at the start that the dynamic linker uses for lazy dispatch, so it turns up as a non-empty section of 0x18 bytes. We should be able to discard it afaict, but given that it does not actually take up any space, it doesn't really matter either way. > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > index a0d94d063fa8..3e628983445a 100644 > > --- a/arch/arm64/Makefile > > +++ b/arch/arm64/Makefile > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > endif > > 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_ARM64_USE_LSE_ATOMICS), y) > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > $(warning LSE atomics not supported by binutils) > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > index 5427f502c3a6..c9ecb3b2007d 100644 > > --- a/arch/arm64/kernel/vmlinux.lds.S > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > @@ -94,7 +94,8 @@ SECTIONS > > /DISCARD/ : { > > *(.interp .dynamic) > > *(.dynsym .dynstr .hash .gnu.hash) > > - *(.eh_frame) > > + *(.plt) *(.data.rel.ro) > > + *(.eh_frame) *(.init.eh_frame) > > Do we need to include .eh_frame_hdr here too? > It would be better to build with -fno-unwind-tables, in which case these sections should not even exist. > > } > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > @@ -209,6 +210,7 @@ SECTIONS > > _data = .; > > _sdata = .; > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > /* > > * Data written with the MMU off but read with the MMU on requires > > @@ -244,6 +246,7 @@ SECTIONS > > _end = .; > > > > STABS_DEBUG > > + DWARF_DEBUG > > I know you didn't add it, but do we _really_ care about stabs debug? Who > generates that for arm64? > > Will ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-23 14:59 ` Ard Biesheuvel 0 siblings, 0 replies; 17+ messages in thread From: Ard Biesheuvel @ 2020-06-23 14:59 UTC (permalink / raw) To: Will Deacon Cc: Mark Rutland, Kees Cook, Arnd Bergmann, Catalin Marinas, Nick Desaulniers, Linux Kernel Mailing List, clang-built-linux, James Morse, Nathan Chancellor, Peter Collingbourne, Linux ARM On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > to discard as it seems that these are still generated even though > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > to the image as it does appear to be mapped near .data. Finally enable > > orphan section warnings. > > Can you elaborate a bit on what .got.plt is being used for, please? I > wonder if there's an interaction with an erratum workaround in the linker > or something. > .got.plt is not used at all, but it has three magic entries at the start that the dynamic linker uses for lazy dispatch, so it turns up as a non-empty section of 0x18 bytes. We should be able to discard it afaict, but given that it does not actually take up any space, it doesn't really matter either way. > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > index a0d94d063fa8..3e628983445a 100644 > > --- a/arch/arm64/Makefile > > +++ b/arch/arm64/Makefile > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > endif > > 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_ARM64_USE_LSE_ATOMICS), y) > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > $(warning LSE atomics not supported by binutils) > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > index 5427f502c3a6..c9ecb3b2007d 100644 > > --- a/arch/arm64/kernel/vmlinux.lds.S > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > @@ -94,7 +94,8 @@ SECTIONS > > /DISCARD/ : { > > *(.interp .dynamic) > > *(.dynsym .dynstr .hash .gnu.hash) > > - *(.eh_frame) > > + *(.plt) *(.data.rel.ro) > > + *(.eh_frame) *(.init.eh_frame) > > Do we need to include .eh_frame_hdr here too? > It would be better to build with -fno-unwind-tables, in which case these sections should not even exist. > > } > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > @@ -209,6 +210,7 @@ SECTIONS > > _data = .; > > _sdata = .; > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > /* > > * Data written with the MMU off but read with the MMU on requires > > @@ -244,6 +246,7 @@ SECTIONS > > _end = .; > > > > STABS_DEBUG > > + DWARF_DEBUG > > I know you didn't add it, but do we _really_ care about stabs debug? Who > generates that for arm64? > > Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-23 14:59 ` Ard Biesheuvel @ 2020-06-23 19:18 ` Nick Desaulniers -1 siblings, 0 replies; 17+ messages in thread From: Nick Desaulniers @ 2020-06-23 19:18 UTC (permalink / raw) To: Ard Biesheuvel Cc: Will Deacon, Kees Cook, Catalin Marinas, Mark Rutland, Arnd Bergmann, Peter Collingbourne, James Morse, Nathan Chancellor, clang-built-linux, Linux ARM, Linux Kernel Mailing List On Tue, Jun 23, 2020 at 7:59 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > to discard as it seems that these are still generated even though > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > to the image as it does appear to be mapped near .data. Finally enable > > > orphan section warnings. > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > wonder if there's an interaction with an erratum workaround in the linker > > or something. > > > > .got.plt is not used at all, but it has three magic entries at the > start that the dynamic linker uses for lazy dispatch, so it turns up > as a non-empty section of 0x18 bytes. Interesting; is there a way to dump those entries? `--dynamic-reloc` flag to objdump? (I suspect the answer might be hexdump...) > We should be able to discard it afaict, but given that it does not > actually take up any space, it doesn't really matter either way. True, but I would prefer to explicitly discard it if we know we're not using it, that way something explicitly breaks if someone tries to make use of it in the future. Then we can consider not discarding it, only if necessary. Modules on arm64 use .got.plt, IIRC? But they have their own linker script so irrelevant I guess. > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > @@ -94,7 +94,8 @@ SECTIONS > > > /DISCARD/ : { > > > *(.interp .dynamic) > > > *(.dynsym .dynstr .hash .gnu.hash) > > > - *(.eh_frame) > > > + *(.plt) *(.data.rel.ro) > > > + *(.eh_frame) *(.init.eh_frame) > > > > Do we need to include .eh_frame_hdr here too? > > > > It would be better to build with -fno-unwind-tables, in which case > these sections should not even exist. Interesting, so we have -fno-asynchronous-unwind-tables and -fno-unwind-tables. Is your suggestion for -fno-unwind-tables a global KBUILD_CFLAG (vs limited to a particular arch)? Interestingly, there a few users of -fasynchronous-unwind-tables in the kernel. vdso's make sense, I think, less sure about the rest. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-23 19:18 ` Nick Desaulniers 0 siblings, 0 replies; 17+ messages in thread From: Nick Desaulniers @ 2020-06-23 19:18 UTC (permalink / raw) To: Ard Biesheuvel Cc: Mark Rutland, Kees Cook, Arnd Bergmann, Peter Collingbourne, Catalin Marinas, Linux Kernel Mailing List, clang-built-linux, James Morse, Nathan Chancellor, Will Deacon, Linux ARM On Tue, Jun 23, 2020 at 7:59 AM Ard Biesheuvel <ardb@kernel.org> wrote: > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > to discard as it seems that these are still generated even though > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > to the image as it does appear to be mapped near .data. Finally enable > > > orphan section warnings. > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > wonder if there's an interaction with an erratum workaround in the linker > > or something. > > > > .got.plt is not used at all, but it has three magic entries at the > start that the dynamic linker uses for lazy dispatch, so it turns up > as a non-empty section of 0x18 bytes. Interesting; is there a way to dump those entries? `--dynamic-reloc` flag to objdump? (I suspect the answer might be hexdump...) > We should be able to discard it afaict, but given that it does not > actually take up any space, it doesn't really matter either way. True, but I would prefer to explicitly discard it if we know we're not using it, that way something explicitly breaks if someone tries to make use of it in the future. Then we can consider not discarding it, only if necessary. Modules on arm64 use .got.plt, IIRC? But they have their own linker script so irrelevant I guess. > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > @@ -94,7 +94,8 @@ SECTIONS > > > /DISCARD/ : { > > > *(.interp .dynamic) > > > *(.dynsym .dynstr .hash .gnu.hash) > > > - *(.eh_frame) > > > + *(.plt) *(.data.rel.ro) > > > + *(.eh_frame) *(.init.eh_frame) > > > > Do we need to include .eh_frame_hdr here too? > > > > It would be better to build with -fno-unwind-tables, in which case > these sections should not even exist. Interesting, so we have -fno-asynchronous-unwind-tables and -fno-unwind-tables. Is your suggestion for -fno-unwind-tables a global KBUILD_CFLAG (vs limited to a particular arch)? Interestingly, there a few users of -fasynchronous-unwind-tables in the kernel. vdso's make sense, I think, less sure about the rest. -- Thanks, ~Nick Desaulniers _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-23 14:59 ` Ard Biesheuvel @ 2020-06-23 21:06 ` Kees Cook -1 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-23 21:06 UTC (permalink / raw) To: Ard Biesheuvel Cc: Will Deacon, Catalin Marinas, Mark Rutland, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Linux ARM, Linux Kernel Mailing List On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > to discard as it seems that these are still generated even though > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > to the image as it does appear to be mapped near .data. Finally enable > > > orphan section warnings. > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > wonder if there's an interaction with an erratum workaround in the linker > > or something. > > > > .got.plt is not used at all, but it has three magic entries at the > start that the dynamic linker uses for lazy dispatch, so it turns up > as a non-empty section of 0x18 bytes. Is there a way to suppress the generation? I haven't found a way, so I'd left it as-is. > We should be able to discard it afaict, but given that it does not > actually take up any space, it doesn't really matter either way. I will add it to the discards then. > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > index a0d94d063fa8..3e628983445a 100644 > > > --- a/arch/arm64/Makefile > > > +++ b/arch/arm64/Makefile > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > endif > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > $(warning LSE atomics not supported by binutils) > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > @@ -94,7 +94,8 @@ SECTIONS > > > /DISCARD/ : { > > > *(.interp .dynamic) > > > *(.dynsym .dynstr .hash .gnu.hash) > > > - *(.eh_frame) > > > + *(.plt) *(.data.rel.ro) > > > + *(.eh_frame) *(.init.eh_frame) > > > > Do we need to include .eh_frame_hdr here too? > > It would be better to build with -fno-unwind-tables, in which case > these sections should not even exist. Nothing seems to help with the .eh_frame issue (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ -I./include/uapi -I./include/generated/uapi -include \ ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame [17] .eh_frame PROGBITS 0000000000000000 000001f0 [18] .rela.eh_frame RELA 0000000000000000 00000618 > > > } > > > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > > @@ -209,6 +210,7 @@ SECTIONS > > > _data = .; > > > _sdata = .; > > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > > > /* > > > * Data written with the MMU off but read with the MMU on requires > > > @@ -244,6 +246,7 @@ SECTIONS > > > _end = .; > > > > > > STABS_DEBUG > > > + DWARF_DEBUG > > > > I know you didn't add it, but do we _really_ care about stabs debug? Who > > generates that for arm64? It's also where .comment and the .symtab ends up living. As a result, I think it's correct to keep it. (If we wanted to split .stabs from .comment/.symtab, we could do that, but I'd kind of like to avoid it for this series, as it feels kind of unrelated.) -- Kees Cook ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-23 21:06 ` Kees Cook 0 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-23 21:06 UTC (permalink / raw) To: Ard Biesheuvel Cc: Mark Rutland, Arnd Bergmann, Peter Collingbourne, Catalin Marinas, Nick Desaulniers, Linux Kernel Mailing List, clang-built-linux, James Morse, Nathan Chancellor, Will Deacon, Linux ARM On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > to discard as it seems that these are still generated even though > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > to the image as it does appear to be mapped near .data. Finally enable > > > orphan section warnings. > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > wonder if there's an interaction with an erratum workaround in the linker > > or something. > > > > .got.plt is not used at all, but it has three magic entries at the > start that the dynamic linker uses for lazy dispatch, so it turns up > as a non-empty section of 0x18 bytes. Is there a way to suppress the generation? I haven't found a way, so I'd left it as-is. > We should be able to discard it afaict, but given that it does not > actually take up any space, it doesn't really matter either way. I will add it to the discards then. > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > index a0d94d063fa8..3e628983445a 100644 > > > --- a/arch/arm64/Makefile > > > +++ b/arch/arm64/Makefile > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > endif > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > $(warning LSE atomics not supported by binutils) > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > @@ -94,7 +94,8 @@ SECTIONS > > > /DISCARD/ : { > > > *(.interp .dynamic) > > > *(.dynsym .dynstr .hash .gnu.hash) > > > - *(.eh_frame) > > > + *(.plt) *(.data.rel.ro) > > > + *(.eh_frame) *(.init.eh_frame) > > > > Do we need to include .eh_frame_hdr here too? > > It would be better to build with -fno-unwind-tables, in which case > these sections should not even exist. Nothing seems to help with the .eh_frame issue (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ -I./include/uapi -I./include/generated/uapi -include \ ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame [17] .eh_frame PROGBITS 0000000000000000 000001f0 [18] .rela.eh_frame RELA 0000000000000000 00000618 > > > } > > > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > > @@ -209,6 +210,7 @@ SECTIONS > > > _data = .; > > > _sdata = .; > > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > > > /* > > > * Data written with the MMU off but read with the MMU on requires > > > @@ -244,6 +246,7 @@ SECTIONS > > > _end = .; > > > > > > STABS_DEBUG > > > + DWARF_DEBUG > > > > I know you didn't add it, but do we _really_ care about stabs debug? Who > > generates that for arm64? It's also where .comment and the .symtab ends up living. As a result, I think it's correct to keep it. (If we wanted to split .stabs from .comment/.symtab, we could do that, but I'd kind of like to avoid it for this series, as it feels kind of unrelated.) -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-23 21:06 ` Kees Cook @ 2020-06-23 21:21 ` Ard Biesheuvel -1 siblings, 0 replies; 17+ messages in thread From: Ard Biesheuvel @ 2020-06-23 21:21 UTC (permalink / raw) To: Kees Cook Cc: Will Deacon, Catalin Marinas, Mark Rutland, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Linux ARM, Linux Kernel Mailing List On Tue, 23 Jun 2020 at 23:06, Kees Cook <keescook@chromium.org> wrote: > > On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > > to discard as it seems that these are still generated even though > > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > > to the image as it does appear to be mapped near .data. Finally enable > > > > orphan section warnings. > > > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > > wonder if there's an interaction with an erratum workaround in the linker > > > or something. > > > > > > > .got.plt is not used at all, but it has three magic entries at the > > start that the dynamic linker uses for lazy dispatch, so it turns up > > as a non-empty section of 0x18 bytes. > > Is there a way to suppress the generation? I haven't found a way, so I'd > left it as-is. > Not really. What we could do is assert that it is empty, and emit it as info, so the 24 bytes are not emitted into the image. This change diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index 6827da7f3aa5..9e13b371559f 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -244,6 +244,9 @@ SECTIONS __pecoff_data_size = ABSOLUTE(. - __initdata_begin); _end = .; + .got.plt (INFO) : { *(.got.plt) } + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, ".got.plt not empty") + STABS_DEBUG HEAD_SYMBOLS results in [28] .bss NOBITS ffff800010d71000 00d70200 0000000000084120 0000000000000000 WA 0 0 4096 [29] .got.plt PROGBITS ffff800010e00000 00d70200 0000000000000018 0000000000000008 W 0 0 8 [30] .comment PROGBITS 0000000000000000 00d70218 000000000000001c 0000000000000001 MS 0 0 1 in the ELF output, so it will be emitted from the image, unless it actually have any entries, in which case we fail the build. > > We should be able to discard it afaict, but given that it does not > > actually take up any space, it doesn't really matter either way. > > I will add it to the discards then. > That would prevent us from doing the assert on its size, so i think the above is more suitable in this case > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > > index a0d94d063fa8..3e628983445a 100644 > > > > --- a/arch/arm64/Makefile > > > > +++ b/arch/arm64/Makefile > > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > > endif > > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > > $(warning LSE atomics not supported by binutils) > > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > > @@ -94,7 +94,8 @@ SECTIONS > > > > /DISCARD/ : { > > > > *(.interp .dynamic) > > > > *(.dynsym .dynstr .hash .gnu.hash) > > > > - *(.eh_frame) > > > > + *(.plt) *(.data.rel.ro) > > > > + *(.eh_frame) *(.init.eh_frame) > > > > > > Do we need to include .eh_frame_hdr here too? > > > > It would be better to build with -fno-unwind-tables, in which case > > these sections should not even exist. > > Nothing seems to help with the .eh_frame issue > (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): > > $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ > -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ > -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ > -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ > -I./include/uapi -I./include/generated/uapi -include \ > ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ > -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ > -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ > -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ > arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S > > $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame > [17] .eh_frame PROGBITS 0000000000000000 000001f0 > [18] .rela.eh_frame RELA 0000000000000000 00000618 > That is because that file has CFI annotations which it doesn't need (since we don't unwind data). The below should fix that - I guess this may have been inherited from 32-bit ARM, where we do use unwind data in the kernel? diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S index 1f93809528a4..d62447964ed9 100644 --- a/arch/arm64/kernel/smccc-call.S +++ b/arch/arm64/kernel/smccc-call.S @@ -9,7 +9,6 @@ #include <asm/assembler.h> .macro SMCCC instr - .cfi_startproc \instr #0 ldr x4, [sp] stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS] @@ -21,7 +20,6 @@ b.ne 1f str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS] 1: ret - .cfi_endproc .endm /* > > > > } > > > > > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > > > @@ -209,6 +210,7 @@ SECTIONS > > > > _data = .; > > > > _sdata = .; > > > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > > > > > /* > > > > * Data written with the MMU off but read with the MMU on requires > > > > @@ -244,6 +246,7 @@ SECTIONS > > > > _end = .; > > > > > > > > STABS_DEBUG > > > > + DWARF_DEBUG > > > > > > I know you didn't add it, but do we _really_ care about stabs debug? Who > > > generates that for arm64? > > It's also where .comment and the .symtab ends up living. As a result, > I think it's correct to keep it. (If we wanted to split .stabs from > .comment/.symtab, we could do that, but I'd kind of like to avoid it for > this series, as it feels kind of unrelated.) > > -- > Kees Cook ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-23 21:21 ` Ard Biesheuvel 0 siblings, 0 replies; 17+ messages in thread From: Ard Biesheuvel @ 2020-06-23 21:21 UTC (permalink / raw) To: Kees Cook Cc: Mark Rutland, Arnd Bergmann, Peter Collingbourne, Catalin Marinas, Nick Desaulniers, Linux Kernel Mailing List, clang-built-linux, James Morse, Nathan Chancellor, Will Deacon, Linux ARM On Tue, 23 Jun 2020 at 23:06, Kees Cook <keescook@chromium.org> wrote: > > On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > > to discard as it seems that these are still generated even though > > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > > to the image as it does appear to be mapped near .data. Finally enable > > > > orphan section warnings. > > > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > > wonder if there's an interaction with an erratum workaround in the linker > > > or something. > > > > > > > .got.plt is not used at all, but it has three magic entries at the > > start that the dynamic linker uses for lazy dispatch, so it turns up > > as a non-empty section of 0x18 bytes. > > Is there a way to suppress the generation? I haven't found a way, so I'd > left it as-is. > Not really. What we could do is assert that it is empty, and emit it as info, so the 24 bytes are not emitted into the image. This change diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S index 6827da7f3aa5..9e13b371559f 100644 --- a/arch/arm64/kernel/vmlinux.lds.S +++ b/arch/arm64/kernel/vmlinux.lds.S @@ -244,6 +244,9 @@ SECTIONS __pecoff_data_size = ABSOLUTE(. - __initdata_begin); _end = .; + .got.plt (INFO) : { *(.got.plt) } + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, ".got.plt not empty") + STABS_DEBUG HEAD_SYMBOLS results in [28] .bss NOBITS ffff800010d71000 00d70200 0000000000084120 0000000000000000 WA 0 0 4096 [29] .got.plt PROGBITS ffff800010e00000 00d70200 0000000000000018 0000000000000008 W 0 0 8 [30] .comment PROGBITS 0000000000000000 00d70218 000000000000001c 0000000000000001 MS 0 0 1 in the ELF output, so it will be emitted from the image, unless it actually have any entries, in which case we fail the build. > > We should be able to discard it afaict, but given that it does not > > actually take up any space, it doesn't really matter either way. > > I will add it to the discards then. > That would prevent us from doing the assert on its size, so i think the above is more suitable in this case > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > > index a0d94d063fa8..3e628983445a 100644 > > > > --- a/arch/arm64/Makefile > > > > +++ b/arch/arm64/Makefile > > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > > endif > > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > > $(warning LSE atomics not supported by binutils) > > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > > @@ -94,7 +94,8 @@ SECTIONS > > > > /DISCARD/ : { > > > > *(.interp .dynamic) > > > > *(.dynsym .dynstr .hash .gnu.hash) > > > > - *(.eh_frame) > > > > + *(.plt) *(.data.rel.ro) > > > > + *(.eh_frame) *(.init.eh_frame) > > > > > > Do we need to include .eh_frame_hdr here too? > > > > It would be better to build with -fno-unwind-tables, in which case > > these sections should not even exist. > > Nothing seems to help with the .eh_frame issue > (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): > > $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ > -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ > -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ > -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ > -I./include/uapi -I./include/generated/uapi -include \ > ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ > -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ > -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ > -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ > arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S > > $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame > [17] .eh_frame PROGBITS 0000000000000000 000001f0 > [18] .rela.eh_frame RELA 0000000000000000 00000618 > That is because that file has CFI annotations which it doesn't need (since we don't unwind data). The below should fix that - I guess this may have been inherited from 32-bit ARM, where we do use unwind data in the kernel? diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S index 1f93809528a4..d62447964ed9 100644 --- a/arch/arm64/kernel/smccc-call.S +++ b/arch/arm64/kernel/smccc-call.S @@ -9,7 +9,6 @@ #include <asm/assembler.h> .macro SMCCC instr - .cfi_startproc \instr #0 ldr x4, [sp] stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS] @@ -21,7 +20,6 @@ b.ne 1f str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS] 1: ret - .cfi_endproc .endm /* > > > > } > > > > > > > > . = KIMAGE_VADDR + TEXT_OFFSET; > > > > @@ -209,6 +210,7 @@ SECTIONS > > > > _data = .; > > > > _sdata = .; > > > > RW_DATA(L1_CACHE_BYTES, PAGE_SIZE, THREAD_ALIGN) > > > > + .got.plt : ALIGN(8) { *(.got.plt) } > > > > > > > > /* > > > > * Data written with the MMU off but read with the MMU on requires > > > > @@ -244,6 +246,7 @@ SECTIONS > > > > _end = .; > > > > > > > > STABS_DEBUG > > > > + DWARF_DEBUG > > > > > > I know you didn't add it, but do we _really_ care about stabs debug? Who > > > generates that for arm64? > > It's also where .comment and the .symtab ends up living. As a result, > I think it's correct to keep it. (If we wanted to split .stabs from > .comment/.symtab, we could do that, but I'd kind of like to avoid it for > this series, as it feels kind of unrelated.) > > -- > Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-23 21:21 ` Ard Biesheuvel @ 2020-06-24 0:05 ` Kees Cook -1 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-24 0:05 UTC (permalink / raw) To: Ard Biesheuvel Cc: Will Deacon, Catalin Marinas, Mark Rutland, Arnd Bergmann, Peter Collingbourne, James Morse, Nick Desaulniers, Nathan Chancellor, clang-built-linux, Linux ARM, Linux Kernel Mailing List On Tue, Jun 23, 2020 at 11:21:16PM +0200, Ard Biesheuvel wrote: > On Tue, 23 Jun 2020 at 23:06, Kees Cook <keescook@chromium.org> wrote: > > > > On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > > > to discard as it seems that these are still generated even though > > > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > > > to the image as it does appear to be mapped near .data. Finally enable > > > > > orphan section warnings. > > > > > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > > > wonder if there's an interaction with an erratum workaround in the linker > > > > or something. > > > > > > > > > > .got.plt is not used at all, but it has three magic entries at the > > > start that the dynamic linker uses for lazy dispatch, so it turns up > > > as a non-empty section of 0x18 bytes. > > > > Is there a way to suppress the generation? I haven't found a way, so I'd > > left it as-is. > > > > Not really. What we could do is assert that it is empty, and emit it > as info, so the 24 bytes are not emitted into the image. > > > This change > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 6827da7f3aa5..9e13b371559f 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -244,6 +244,9 @@ SECTIONS > __pecoff_data_size = ABSOLUTE(. - __initdata_begin); > _end = .; > > + .got.plt (INFO) : { *(.got.plt) } > + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, > ".got.plt not empty") > + Oh yes, I like that. I will do so. > STABS_DEBUG > > HEAD_SYMBOLS > > results in > > [28] .bss NOBITS ffff800010d71000 00d70200 > 0000000000084120 0000000000000000 WA 0 0 4096 > [29] .got.plt PROGBITS ffff800010e00000 00d70200 > 0000000000000018 0000000000000008 W 0 0 8 > [30] .comment PROGBITS 0000000000000000 00d70218 > 000000000000001c 0000000000000001 MS 0 0 1 > > in the ELF output, so it will be emitted from the image, unless it > actually have any entries, in which case we fail the build. > > > > > > We should be able to discard it afaict, but given that it does not > > > actually take up any space, it doesn't really matter either way. > > > > I will add it to the discards then. > > > > That would prevent us from doing the assert on its size, so i think > the above is more suitable in this case > > > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > > > index a0d94d063fa8..3e628983445a 100644 > > > > > --- a/arch/arm64/Makefile > > > > > +++ b/arch/arm64/Makefile > > > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > > > endif > > > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > > > $(warning LSE atomics not supported by binutils) > > > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > > > @@ -94,7 +94,8 @@ SECTIONS > > > > > /DISCARD/ : { > > > > > *(.interp .dynamic) > > > > > *(.dynsym .dynstr .hash .gnu.hash) > > > > > - *(.eh_frame) > > > > > + *(.plt) *(.data.rel.ro) > > > > > + *(.eh_frame) *(.init.eh_frame) > > > > > > > > Do we need to include .eh_frame_hdr here too? > > > > > > It would be better to build with -fno-unwind-tables, in which case > > > these sections should not even exist. > > > > Nothing seems to help with the .eh_frame issue > > (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): > > > > $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ > > -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ > > -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ > > -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ > > -I./include/uapi -I./include/generated/uapi -include \ > > ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ > > -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ > > -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ > > -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ > > arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S > > > > $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame > > [17] .eh_frame PROGBITS 0000000000000000 000001f0 > > [18] .rela.eh_frame RELA 0000000000000000 00000618 > > > > That is because that file has CFI annotations which it doesn't need > (since we don't unwind data). Oh no, another TLA collision. ;) "Call Frame Information". Nice find. I will fix this as you've suggested too. > The below should fix that - I guess this may have been inherited from > 32-bit ARM, where we do use unwind data in the kernel? > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S > index 1f93809528a4..d62447964ed9 100644 > --- a/arch/arm64/kernel/smccc-call.S > +++ b/arch/arm64/kernel/smccc-call.S > @@ -9,7 +9,6 @@ > #include <asm/assembler.h> > > .macro SMCCC instr > - .cfi_startproc > \instr #0 > ldr x4, [sp] > stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS] > @@ -21,7 +20,6 @@ > b.ne 1f > str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS] > 1: ret > - .cfi_endproc > .endm -- Kees Cook ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement @ 2020-06-24 0:05 ` Kees Cook 0 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-24 0:05 UTC (permalink / raw) To: Ard Biesheuvel Cc: Mark Rutland, Arnd Bergmann, Peter Collingbourne, Catalin Marinas, Nick Desaulniers, Linux Kernel Mailing List, clang-built-linux, James Morse, Nathan Chancellor, Will Deacon, Linux ARM On Tue, Jun 23, 2020 at 11:21:16PM +0200, Ard Biesheuvel wrote: > On Tue, 23 Jun 2020 at 23:06, Kees Cook <keescook@chromium.org> wrote: > > > > On Tue, Jun 23, 2020 at 04:59:39PM +0200, Ard Biesheuvel wrote: > > > On Tue, 23 Jun 2020 at 16:52, Will Deacon <will@kernel.org> wrote: > > > > > > > > On Mon, Jun 22, 2020 at 01:58:15PM -0700, Kees Cook 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. > > > > > > > > > > Explicitly include debug sections when they're present. Add .eh_frame* > > > > > to discard as it seems that these are still generated even though > > > > > -fno-asynchronous-unwind-tables is being specified. Add .plt and > > > > > .data.rel.ro to discards as they are not actually used. Add .got.plt > > > > > to the image as it does appear to be mapped near .data. Finally enable > > > > > orphan section warnings. > > > > > > > > Can you elaborate a bit on what .got.plt is being used for, please? I > > > > wonder if there's an interaction with an erratum workaround in the linker > > > > or something. > > > > > > > > > > .got.plt is not used at all, but it has three magic entries at the > > > start that the dynamic linker uses for lazy dispatch, so it turns up > > > as a non-empty section of 0x18 bytes. > > > > Is there a way to suppress the generation? I haven't found a way, so I'd > > left it as-is. > > > > Not really. What we could do is assert that it is empty, and emit it > as info, so the 24 bytes are not emitted into the image. > > > This change > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > index 6827da7f3aa5..9e13b371559f 100644 > --- a/arch/arm64/kernel/vmlinux.lds.S > +++ b/arch/arm64/kernel/vmlinux.lds.S > @@ -244,6 +244,9 @@ SECTIONS > __pecoff_data_size = ABSOLUTE(. - __initdata_begin); > _end = .; > > + .got.plt (INFO) : { *(.got.plt) } > + ASSERT(SIZEOF(.got.plt) == 0 || SIZEOF(.got.plt) == 0x18, > ".got.plt not empty") > + Oh yes, I like that. I will do so. > STABS_DEBUG > > HEAD_SYMBOLS > > results in > > [28] .bss NOBITS ffff800010d71000 00d70200 > 0000000000084120 0000000000000000 WA 0 0 4096 > [29] .got.plt PROGBITS ffff800010e00000 00d70200 > 0000000000000018 0000000000000008 W 0 0 8 > [30] .comment PROGBITS 0000000000000000 00d70218 > 000000000000001c 0000000000000001 MS 0 0 1 > > in the ELF output, so it will be emitted from the image, unless it > actually have any entries, in which case we fail the build. > > > > > > We should be able to discard it afaict, but given that it does not > > > actually take up any space, it doesn't really matter either way. > > > > I will add it to the discards then. > > > > That would prevent us from doing the assert on its size, so i think > the above is more suitable in this case > > > > > > diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile > > > > > index a0d94d063fa8..3e628983445a 100644 > > > > > --- a/arch/arm64/Makefile > > > > > +++ b/arch/arm64/Makefile > > > > > @@ -29,6 +29,10 @@ LDFLAGS_vmlinux += --fix-cortex-a53-843419 > > > > > endif > > > > > 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_ARM64_USE_LSE_ATOMICS), y) > > > > > ifneq ($(CONFIG_ARM64_LSE_ATOMICS), y) > > > > > $(warning LSE atomics not supported by binutils) > > > > > diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S > > > > > index 5427f502c3a6..c9ecb3b2007d 100644 > > > > > --- a/arch/arm64/kernel/vmlinux.lds.S > > > > > +++ b/arch/arm64/kernel/vmlinux.lds.S > > > > > @@ -94,7 +94,8 @@ SECTIONS > > > > > /DISCARD/ : { > > > > > *(.interp .dynamic) > > > > > *(.dynsym .dynstr .hash .gnu.hash) > > > > > - *(.eh_frame) > > > > > + *(.plt) *(.data.rel.ro) > > > > > + *(.eh_frame) *(.init.eh_frame) > > > > > > > > Do we need to include .eh_frame_hdr here too? > > > > > > It would be better to build with -fno-unwind-tables, in which case > > > these sections should not even exist. > > > > Nothing seems to help with the .eh_frame issue > > (even with -fno-asynchronous-unwind-tables and -fno-unwind-tables): > > > > $ aarch64-linux-gnu-gcc -Wp,-MMD,arch/arm64/kernel/.smccc-call.o.d \ > > -nostdinc -isystem /usr/lib/gcc-cross/aarch64-linux-gnu/9/include \ > > -I./arch/arm64/include -I./arch/arm64/include/generated -I./include \ > > -I./arch/arm64/include/uapi -I./arch/arm64/include/generated/uapi \ > > -I./include/uapi -I./include/generated/uapi -include \ > > ./include/linux/kconfig.h -D__KERNEL__ -mlittle-endian \ > > -DCC_USING_PATCHABLE_FUNCTION_ENTRY -DKASAN_SHADOW_SCALE_SHIFT=3 \ > > -D__ASSEMBLY__ -fno-PIE -mabi=lp64 -fno-asynchronous-unwind-tables \ > > -fno-unwind-tables -DKASAN_SHADOW_SCALE_SHIFT=3 -Wa,-gdwarf-2 -c -o \ > > arch/arm64/kernel/smccc-call.o arch/arm64/kernel/smccc-call.S > > > > $ readelf -S arch/arm64/kernel/smccc-call.o | grep eh_frame > > [17] .eh_frame PROGBITS 0000000000000000 000001f0 > > [18] .rela.eh_frame RELA 0000000000000000 00000618 > > > > That is because that file has CFI annotations which it doesn't need > (since we don't unwind data). Oh no, another TLA collision. ;) "Call Frame Information". Nice find. I will fix this as you've suggested too. > The below should fix that - I guess this may have been inherited from > 32-bit ARM, where we do use unwind data in the kernel? > > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S > index 1f93809528a4..d62447964ed9 100644 > --- a/arch/arm64/kernel/smccc-call.S > +++ b/arch/arm64/kernel/smccc-call.S > @@ -9,7 +9,6 @@ > #include <asm/assembler.h> > > .macro SMCCC instr > - .cfi_startproc > \instr #0 > ldr x4, [sp] > stp x0, x1, [x4, #ARM_SMCCC_RES_X0_OFFS] > @@ -21,7 +20,6 @@ > b.ne 1f > str x6, [x4, ARM_SMCCC_QUIRK_STATE_OFFS] > 1: ret > - .cfi_endproc > .endm -- Kees Cook _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-22 20:58 ` [PATCH v2 2/2] arm64/build: Warn on orphan section placement Kees Cook 2020-06-23 14:52 ` Will Deacon @ 2020-06-24 3:52 ` kernel test robot 2020-06-24 4:41 ` Kees Cook 1 sibling, 1 reply; 17+ messages in thread From: kernel test robot @ 2020-06-24 3:52 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 12081 bytes --] Hi Kees, I love your patch! Perhaps something to improve: [auto build test WARNING on arm64/for-next/core] [cannot apply to arm-perf/for-next/perf] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Kees-Cook/arm64-Warn-on-orphan-section-placement/20200623-050132 base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core config: arm64-randconfig-r003-20200623 (attached as .config) compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All warnings (new ones prefixed by >>): ld.lld: warning: arch/arm64/built-in.a(mm/mmu.o):(".mmuoff.data.write") is being placed in '".mmuoff.data.write"' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm64-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(efi-stub-helper.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(file.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(gop.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-cmdline.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-ctype.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_empty_tree.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_ro.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_rw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_sw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_wip.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(mem.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(pci.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(random.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(randomalloc.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(secureboot.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(skip_spaces.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' >> ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(string.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(tpm.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab' ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab' ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab' ld.lld: warning: arch/arm64/built-in.a(mm/mmu.o):(".mmuoff.data.write") is being placed in '".mmuoff.data.write"' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm64-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(efi-stub-helper.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(file.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(gop.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-cmdline.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-ctype.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_empty_tree.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_ro.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_rw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_sw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_wip.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(mem.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(pci.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(random.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(randomalloc.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(secureboot.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(skip_spaces.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(string.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(tpm.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab' ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab' ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab' ld.lld: warning: arch/arm64/built-in.a(mm/mmu.o):(".mmuoff.data.write") is being placed in '".mmuoff.data.write"' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm64-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(efi-stub-helper.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(file.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(gop.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-cmdline.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-ctype.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_empty_tree.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_ro.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_rw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_sw.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(lib-fdt_wip.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(mem.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(pci.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(random.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(randomalloc.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(secureboot.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(skip_spaces.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(string.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(tpm.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' ld.lld: warning: <internal>:(.symtab) is being placed in '.symtab' ld.lld: warning: <internal>:(.shstrtab) is being placed in '.shstrtab' ld.lld: warning: <internal>:(.strtab) is being placed in '.strtab' --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 40798 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/2] arm64/build: Warn on orphan section placement 2020-06-24 3:52 ` kernel test robot @ 2020-06-24 4:41 ` Kees Cook 0 siblings, 0 replies; 17+ messages in thread From: Kees Cook @ 2020-06-24 4:41 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1839 bytes --] On Wed, Jun 24, 2020 at 11:52:33AM +0800, kernel test robot wrote: > Hi Kees, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on arm64/for-next/core] > [cannot apply to arm-perf/for-next/perf] > [If your patch is applied to the wrong git tree, kindly drop us a note. > And when submitting patch, we suggest to use as documented in > https://git-scm.com/docs/git-format-patch] > > url: https://github.com/0day-ci/linux/commits/Kees-Cook/arm64-Warn-on-orphan-section-placement/20200623-050132 > base: https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/core > config: arm64-randconfig-r003-20200623 (attached as .config) > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34) > reproduce (this is a W=1 build): > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # install arm64 cross compiling tool for clang build > # apt-get install binutils-aarch64-linux-gnu > # save the attached .config to linux build tree > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 > > If you fix the issue, kindly add following tag as appropriate > Reported-by: kernel test robot <lkp@intel.com> > > All warnings (new ones prefixed by >>): > > ld.lld: warning: arch/arm64/built-in.a(mm/mmu.o):(".mmuoff.data.write") is being placed in '".mmuoff.data.write"' > ld.lld: warning: ./drivers/firmware/efi/libstub/lib.a(arm-stub.stub.o):(.init.note.gnu.property) is being placed in '.init.note.gnu.property' Thanks! This is all fixed in the v3 series now: https://lore.kernel.org/lkml/20200624014940.1204448-1-keescook(a)chromium.org/ -- Kees Cook ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-06-24 4:41 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-22 20:58 [PATCH v2 0/2] arm64: Warn on orphan section placement Kees Cook 2020-06-22 20:58 ` [PATCH v2 1/2] arm64/build: Use common DISCARDS in linker script Kees Cook 2020-06-22 20:58 ` [PATCH v2 2/2] arm64/build: Warn on orphan section placement Kees Cook 2020-06-23 14:52 ` Will Deacon 2020-06-23 14:52 ` Will Deacon 2020-06-23 14:59 ` Ard Biesheuvel 2020-06-23 14:59 ` Ard Biesheuvel 2020-06-23 19:18 ` Nick Desaulniers 2020-06-23 19:18 ` Nick Desaulniers 2020-06-23 21:06 ` Kees Cook 2020-06-23 21:06 ` Kees Cook 2020-06-23 21:21 ` Ard Biesheuvel 2020-06-23 21:21 ` Ard Biesheuvel 2020-06-24 0:05 ` Kees Cook 2020-06-24 0:05 ` Kees Cook 2020-06-24 3:52 ` kernel test robot 2020-06-24 4:41 ` Kees Cook
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.