2020-06-24 01:53:34

by Kees Cook

[permalink] [raw]
Subject: [PATCH v3 4/9] x86/build: Warn on orphan section placement

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

Discards the unused rela, plt, and got sections that are not needed
in the final vmlinux, stop emitting kprobe sections without kprobes,
and enable orphan section warnings.

Signed-off-by: Kees Cook <[email protected]>
---
arch/x86/Makefile | 4 ++++
arch/x86/include/asm/asm.h | 6 +++++-
arch/x86/kernel/vmlinux.lds.S | 6 ++++++
3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 00e378de8bc0..f8a5b2333729 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -51,6 +51,10 @@ ifdef CONFIG_X86_NEED_RELOCS
LDFLAGS_vmlinux := --emit-relocs --discard-none
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
+
#
# Prevent GCC from generating any FP code by mistake.
#
diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
index 0f63585edf5f..92feec0f0a12 100644
--- a/arch/x86/include/asm/asm.h
+++ b/arch/x86/include/asm/asm.h
@@ -138,11 +138,15 @@
# define _ASM_EXTABLE_FAULT(from, to) \
_ASM_EXTABLE_HANDLE(from, to, ex_handler_fault)

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

#else
# define _EXPAND_EXTABLE_HANDLE(x) #x
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 3bfc8dd8a43d..bb085ceeaaad 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -412,6 +412,12 @@ SECTIONS
DWARF_DEBUG

DISCARDS
+ /DISCARD/ : {
+ *(.rela.*) *(.rela_*)
+ *(.rel.*) *(.rel_*)
+ *(.got) *(.got.*)
+ *(.igot.*) *(.iplt)
+ }
}


--
2.25.1


2020-06-27 15:49:21

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3 4/9] x86/build: Warn on orphan section placement

On Thu, Jun 25, 2020 at 02:36:27AM +0800, kernel test robot wrote:
> I love your patch! Perhaps something to improve:
> [...]
> config: x86_64-randconfig-a012-20200624 (attached as .config)

CONFIG_KCSAN=y

> compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)
> [...]
> All warnings (new ones prefixed by >>):
>
> ld.lld: warning: drivers/built-in.a(mfd/mt6397-irq.o):(.init_array.0) is being placed in '.init_array.0'

As far as I can tell, this is a Clang bug. But I don't know the
internals here, so I've opened:
https://bugs.llvm.org/show_bug.cgi?id=46478

and created a work-around patch for the kernel:


commit 915f2c343e59a14f00c68f4d7afcfdc621de0674
Author: Kees Cook <[email protected]>
Date: Sat Jun 27 08:07:54 2020 -0700

vmlinux.lds.h: Avoid KCSAN's unwanted sections

KCSAN (-fsanitize=thread) produces unwanted[1] .eh_frame and .init_array.*
sections. Add them to DISCARDS, except with CONFIG_CONSTRUCTORS, which
wants to keep .init_array.* sections.

[1] https://bugs.llvm.org/show_bug.cgi?id=46478

Signed-off-by: Kees Cook <[email protected]>

diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index f8a5b2333729..41c8c73de6c4 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -195,7 +195,9 @@ endif
# Workaround for a gcc prelease that unfortunately was shipped in a suse release
KBUILD_CFLAGS += -Wno-sign-compare
#
-KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
+KBUILD_AFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
+KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
+KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)

# Avoid indirect branches in kernel to deal with Spectre
ifdef CONFIG_RETPOLINE
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index b1dca0762fc5..a44ee16abc78 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -934,10 +934,28 @@
EXIT_DATA
#endif

+/*
+ * Clang's -fsanitize=thread produces unwanted sections (.eh_frame
+ * and .init_array.*), but CONFIG_CONSTRUCTORS wants to keep any
+ * .init_array.* sections.
+ * https://bugs.llvm.org/show_bug.cgi?id=46478
+ */
+#if defined(CONFIG_KCSAN) && !defined(CONFIG_CONSTRUCTORS)
+#define KCSAN_DISCARDS \
+ *(.init_array) *(.init_array.*) \
+ *(.eh_frame)
+#elif defined(CONFIG_KCSAN) && defined(CONFIG_CONSTRUCTORS)
+#define KCSAN_DISCARDS \
+ *(.eh_frame)
+#else
+#define KCSAN_DISCARDS
+#endif
+
#define DISCARDS \
/DISCARD/ : { \
EXIT_DISCARDS \
EXIT_CALL \
+ KCSAN_DISCARDS \
*(.discard) \
*(.discard.*) \
*(.modinfo) \

--
Kees Cook

2020-06-29 19:05:06

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH v3 4/9] x86/build: Warn on orphan section placement

On Mon, Jun 29, 2020 at 04:54:13PM +0200, Marco Elver wrote:
> On Sat, 27 Jun 2020 at 17:44, Kees Cook <[email protected]> wrote:
> >
> > On Thu, Jun 25, 2020 at 02:36:27AM +0800, kernel test robot wrote:
> > > I love your patch! Perhaps something to improve:
> > > [...]
> > > config: x86_64-randconfig-a012-20200624 (attached as .config)
> >
> > CONFIG_KCSAN=y
> >
> > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)
> > > [...]
> > > All warnings (new ones prefixed by >>):
> > >
> > > ld.lld: warning: drivers/built-in.a(mfd/mt6397-irq.o):(.init_array.0) is being placed in '.init_array.0'
> >
> > As far as I can tell, this is a Clang bug. But I don't know the
> > internals here, so I've opened:
> > https://bugs.llvm.org/show_bug.cgi?id=46478
> >
> > and created a work-around patch for the kernel:
>
> Thanks, minor comments below.
>
> With KCSAN this is:
>
> Tested-by: Marco Elver <[email protected]>

Thanks!

>
>
> > commit 915f2c343e59a14f00c68f4d7afcfdc621de0674
> > Author: Kees Cook <[email protected]>
> > Date: Sat Jun 27 08:07:54 2020 -0700
> >
> > vmlinux.lds.h: Avoid KCSAN's unwanted sections
>
> Since you found that it's also KASAN, this probably wants updating.

Yeah, I found that while testing the v4 series and updated the patch
there.

> > KCSAN (-fsanitize=thread) produces unwanted[1] .eh_frame and .init_array.*
> > sections. Add them to DISCARDS, except with CONFIG_CONSTRUCTORS, which
> > wants to keep .init_array.* sections.
> >
> > [1] https://bugs.llvm.org/show_bug.cgi?id=46478
> >
> > Signed-off-by: Kees Cook <[email protected]>
> >
> > diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > index f8a5b2333729..41c8c73de6c4 100644
> > --- a/arch/x86/Makefile
> > +++ b/arch/x86/Makefile
> > @@ -195,7 +195,9 @@ endif
> > # Workaround for a gcc prelease that unfortunately was shipped in a suse release
> > KBUILD_CFLAGS += -Wno-sign-compare
> > #
> > -KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> > +KBUILD_AFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
> > +KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
> > +KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)
>
> Why are they needed? They are not mentioned in the commit message.

This was a mis-applied chunk (I also noticed this in the v4).

> > +/*
> > + * Clang's -fsanitize=thread produces unwanted sections (.eh_frame
> > + * and .init_array.*), but CONFIG_CONSTRUCTORS wants to keep any
> > + * .init_array.* sections.
> > + * https://bugs.llvm.org/show_bug.cgi?id=46478
> > + */
> > +#if defined(CONFIG_KCSAN) && !defined(CONFIG_CONSTRUCTORS)
>
> CONFIG_KASAN as well?
>
> > +#define KCSAN_DISCARDS \
> > + *(.init_array) *(.init_array.*) \
> > + *(.eh_frame)
> > +#elif defined(CONFIG_KCSAN) && defined(CONFIG_CONSTRUCTORS)
> > +#define KCSAN_DISCARDS \
> > + *(.eh_frame)
> > +#else
> > +#define KCSAN_DISCARDS
> > +#endif
> > +
> > #define DISCARDS \
> > /DISCARD/ : { \
> > EXIT_DISCARDS \
> > EXIT_CALL \
> > + KCSAN_DISCARDS \
>
> Maybe just 'SANITIZER_DISCARDS'?

Sure! I will rename it.

>
> > *(.discard) \
> > *(.discard.*) \
> > *(.modinfo) \
> >
> > --
> > Kees Cook

--
Kees Cook

2020-06-29 19:44:10

by Marco Elver

[permalink] [raw]
Subject: Re: [PATCH v3 4/9] x86/build: Warn on orphan section placement

On Sat, 27 Jun 2020 at 17:44, Kees Cook <[email protected]> wrote:
>
> On Thu, Jun 25, 2020 at 02:36:27AM +0800, kernel test robot wrote:
> > I love your patch! Perhaps something to improve:
> > [...]
> > config: x86_64-randconfig-a012-20200624 (attached as .config)
>
> CONFIG_KCSAN=y
>
> > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 1d4c87335d5236ea1f35937e1014980ba961ae34)
> > [...]
> > All warnings (new ones prefixed by >>):
> >
> > ld.lld: warning: drivers/built-in.a(mfd/mt6397-irq.o):(.init_array.0) is being placed in '.init_array.0'
>
> As far as I can tell, this is a Clang bug. But I don't know the
> internals here, so I've opened:
> https://bugs.llvm.org/show_bug.cgi?id=46478
>
> and created a work-around patch for the kernel:

Thanks, minor comments below.

With KCSAN this is:

Tested-by: Marco Elver <[email protected]>


> commit 915f2c343e59a14f00c68f4d7afcfdc621de0674
> Author: Kees Cook <[email protected]>
> Date: Sat Jun 27 08:07:54 2020 -0700
>
> vmlinux.lds.h: Avoid KCSAN's unwanted sections

Since you found that it's also KASAN, this probably wants updating.

> KCSAN (-fsanitize=thread) produces unwanted[1] .eh_frame and .init_array.*
> sections. Add them to DISCARDS, except with CONFIG_CONSTRUCTORS, which
> wants to keep .init_array.* sections.
>
> [1] https://bugs.llvm.org/show_bug.cgi?id=46478
>
> Signed-off-by: Kees Cook <[email protected]>
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index f8a5b2333729..41c8c73de6c4 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -195,7 +195,9 @@ endif
> # Workaround for a gcc prelease that unfortunately was shipped in a suse release
> KBUILD_CFLAGS += -Wno-sign-compare
> #
> -KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
> +KBUILD_AFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
> +KBUILD_CFLAGS += -fno-asynchronous-unwind-tables -fno-unwind-tables
> +KBUILD_LDFLAGS += $(call ld-option,--no-ld-generated-unwind-info)

Why are they needed? They are not mentioned in the commit message.

> # Avoid indirect branches in kernel to deal with Spectre
> ifdef CONFIG_RETPOLINE
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index b1dca0762fc5..a44ee16abc78 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -934,10 +934,28 @@
> EXIT_DATA
> #endif
>
> +/*
> + * Clang's -fsanitize=thread produces unwanted sections (.eh_frame
> + * and .init_array.*), but CONFIG_CONSTRUCTORS wants to keep any
> + * .init_array.* sections.
> + * https://bugs.llvm.org/show_bug.cgi?id=46478
> + */
> +#if defined(CONFIG_KCSAN) && !defined(CONFIG_CONSTRUCTORS)

CONFIG_KASAN as well?

> +#define KCSAN_DISCARDS \
> + *(.init_array) *(.init_array.*) \
> + *(.eh_frame)
> +#elif defined(CONFIG_KCSAN) && defined(CONFIG_CONSTRUCTORS)
> +#define KCSAN_DISCARDS \
> + *(.eh_frame)
> +#else
> +#define KCSAN_DISCARDS
> +#endif
> +
> #define DISCARDS \
> /DISCARD/ : { \
> EXIT_DISCARDS \
> EXIT_CALL \
> + KCSAN_DISCARDS \

Maybe just 'SANITIZER_DISCARDS'?

> *(.discard) \
> *(.discard.*) \
> *(.modinfo) \
>
> --
> Kees Cook