[v4,34/75] x86/head/64: Build k/head64.c with -fno-stack-protector

Commit Message

Joerg Roedel July 14, 2020, 12:08 p.m. UTC

From: Joerg Roedel <[e-mail]>

The code inserted by the stack protector does not work in the early
boot environment because it uses the GS segment, at least with memory
encryption enabled. Make sure the early code is compiled without this
feature enabled.

Signed-off-by: Joerg Roedel <[e-mail]>
---
 arch/x86/kernel/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Kees Cook July 15, 2020, 1:34 a.m. UTC | #1

On Tue, Jul 14, 2020 at 02:08:36PM +0200, Joerg Roedel wrote:
> From: Joerg Roedel <[e-mail]>
> 
> The code inserted by the stack protector does not work in the early
> boot environment because it uses the GS segment, at least with memory
> encryption enabled. Make sure the early code is compiled without this
> feature enabled.
> 
> Signed-off-by: Joerg Roedel <[e-mail]>
> ---
>  arch/x86/kernel/Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index e77261db2391..1b166b866059 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -39,6 +39,10 @@ ifdef CONFIG_FRAME_POINTER
>  OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o		:= y
>  endif
>  
> +# make sure head64.c is built without stack protector
> +nostackp := $(call cc-option, -fno-stack-protector)
> +CFLAGS_head64.o		:= $(nostackp)

Recent refactoring[1] for stack protector suggests this should just
unconditionally be:

CFLAGS_head64.o			+= -fno-stack-protector

But otherwise, yeah, this should be fine here -- it's all early init
stuff.

Reviewed-by: Kees Cook <[e-mail]>

[1] https://lore.kernel.org/lkml/[e-mail]/

> +
>  # If instrumentation of this dir is enabled, boot hangs during first second.
>  # Probably could be more selective here, but note that files related to irqs,
>  # boot, dumpstack/stacktrace, etc are either non-interesting or can lead to
> -- 
> 2.27.0
>

Joerg Roedel July 15, 2020, 4:34 p.m. UTC | #2

On Tue, Jul 14, 2020 at 06:34:24PM -0700, Kees Cook wrote:
> On Tue, Jul 14, 2020 at 02:08:36PM +0200, Joerg Roedel wrote:
> > +# make sure head64.c is built without stack protector
> > +nostackp := $(call cc-option, -fno-stack-protector)
> > +CFLAGS_head64.o		:= $(nostackp)
> 
> Recent refactoring[1] for stack protector suggests this should just
> unconditionally be:
> 
> CFLAGS_head64.o			+= -fno-stack-protector
> 
> But otherwise, yeah, this should be fine here -- it's all early init
> stuff.
> 
> Reviewed-by: Kees Cook <[e-mail]>

Thanks, I am not sure this patch will be needed in the next version, as
I am currently rebasing to tip/master, which also made idt_descr static
in kernel/idt.c.

So with that I think I have to move the early IDT init functions to
kernel/idt.c too and setup %gs earlier in head_64.S to make
stack-protector happy.

The %gs setup actually needs to happen two times, one time when the
kernel still runs identity mapped and then again when it switched to
virtual addresses.

Regards,

	Joerg

Patch

diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index e77261db2391..1b166b866059 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -39,6 +39,10 @@  ifdef CONFIG_FRAME_POINTER
 OBJECT_FILES_NON_STANDARD_ftrace_$(BITS).o		:= y
 endif
 
+# make sure head64.c is built without stack protector
+nostackp := $(call cc-option, -fno-stack-protector)
+CFLAGS_head64.o		:= $(nostackp)
+
 # If instrumentation of this dir is enabled, boot hangs during first second.
 # Probably could be more selective here, but note that files related to irqs,
 # boot, dumpstack/stacktrace, etc are either non-interesting or can lead to