| Message ID | 20190725200625.174838-2-ndesaulniers@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/2] x86/purgatory: do not use __builtin_memcpy and __builtin_memset | expand |
On Thu, 25 Jul 2019, Nick Desaulniers wrote: > KBUILD_CFLAGS is very carefully built up in the top level Makefile, > particularly when cross compiling or using different build tools. > Resetting KBUILD_CFLAGS via := assignment is an antipattern. > > The comment above the reset mentions that -pg is problematic. Other > Makefiles use `CFLAGS_REMOVE_file.o = $(CC_FLAGS_FTRACE)` when > CONFIG_FUNCTION_TRACER is set. Prefer that pattern to wiping out all of > the important KBUILD_CFLAGS then manually having to re-add them. Seems > also that __stack_chk_fail references are generated when using > CONFIG_STACKPROTECTOR or CONFIG_STACKPROTECTOR_STRONG. Looking at the resulting build flags. Most stuff looks correct but there are a few which need to be looked at twice. removes: -ffreestanding -fno-builtin -fno-zero-initialized-in-bss changes: -mcmodel=large to -mcmodel=kernel adds: -mindirect-branch-register -mindirect-branch=thunk-extern The latter makes me nervous. That probably wants to have retpoline disabled as well. It's not having an instance right now, but ... Thanks, tglx
On Fri, 26 Jul 2019, Thomas Gleixner wrote: Ping... > On Thu, 25 Jul 2019, Nick Desaulniers wrote: > > > KBUILD_CFLAGS is very carefully built up in the top level Makefile, > > particularly when cross compiling or using different build tools. > > Resetting KBUILD_CFLAGS via := assignment is an antipattern. > > > > The comment above the reset mentions that -pg is problematic. Other > > Makefiles use `CFLAGS_REMOVE_file.o = $(CC_FLAGS_FTRACE)` when > > CONFIG_FUNCTION_TRACER is set. Prefer that pattern to wiping out all of > > the important KBUILD_CFLAGS then manually having to re-add them. Seems > > also that __stack_chk_fail references are generated when using > > CONFIG_STACKPROTECTOR or CONFIG_STACKPROTECTOR_STRONG. > > Looking at the resulting build flags. Most stuff looks correct but there > are a few which need to be looked at twice. > > removes: > > -ffreestanding > -fno-builtin > -fno-zero-initialized-in-bss > > changes: > > -mcmodel=large to -mcmodel=kernel > > adds: > > -mindirect-branch-register > -mindirect-branch=thunk-extern > > The latter makes me nervous. That probably wants to have retpoline disabled > as well. It's not having an instance right now, but ... > > Thanks, > > tglx >
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile index 91ef244026d2..940f043a4d97 100644 --- a/arch/x86/purgatory/Makefile +++ b/arch/x86/purgatory/Makefile @@ -20,11 +20,27 @@ KCOV_INSTRUMENT := n # Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That # in turn leaves some undefined symbols like __fentry__ in purgatory and not -# sure how to relocate those. Like kexec-tools, use custom flags. - -KBUILD_CFLAGS := -fno-strict-aliasing -Wall -Wstrict-prototypes -fno-zero-initialized-in-bss -fno-builtin -ffreestanding -c -Os -mcmodel=large -KBUILD_CFLAGS += -m$(BITS) -KBUILD_CFLAGS += $(call cc-option,-fno-PIE) +# sure how to relocate those. +ifdef CONFIG_FUNCTION_TRACER +CFLAGS_REMOVE_sha256.o += $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_purgatory.o += $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_string.o += $(CC_FLAGS_FTRACE) +CFLAGS_REMOVE_kexec-purgatory.o += $(CC_FLAGS_FTRACE) +endif + +ifdef CONFIG_STACKPROTECTOR +CFLAGS_REMOVE_sha256.o += -fstack-protector +CFLAGS_REMOVE_purgatory.o += -fstack-protector +CFLAGS_REMOVE_string.o += -fstack-protector +CFLAGS_REMOVE_kexec-purgatory.o += -fstack-protector +endif + +ifdef CONFIG_STACKPROTECTOR_STRONG +CFLAGS_REMOVE_sha256.o += -fstack-protector-strong +CFLAGS_REMOVE_purgatory.o += -fstack-protector-strong +CFLAGS_REMOVE_string.o += -fstack-protector-strong +CFLAGS_REMOVE_kexec-purgatory.o += -fstack-protector-strong +endif $(obj)/purgatory.ro: $(PURGATORY_OBJS) FORCE $(call if_changed,ld)
KBUILD_CFLAGS is very carefully built up in the top level Makefile, particularly when cross compiling or using different build tools. Resetting KBUILD_CFLAGS via := assignment is an antipattern. The comment above the reset mentions that -pg is problematic. Other Makefiles use `CFLAGS_REMOVE_file.o = $(CC_FLAGS_FTRACE)` when CONFIG_FUNCTION_TRACER is set. Prefer that pattern to wiping out all of the important KBUILD_CFLAGS then manually having to re-add them. Seems also that __stack_chk_fail references are generated when using CONFIG_STACKPROTECTOR or CONFIG_STACKPROTECTOR_STRONG. Fixes: 8fc5b4d4121c ("purgatory: core purgatory functionality") Reported-by: Vaibhav Rustagi <vaibhavrustagi@google.com> Suggested-by: Peter Zijlstra <peterz@infradead.org> Suggested-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Tested-by: Vaibhav Rustagi <vaibhavrustagi@google.com> --- Changes v3 -> v4: * Use tabs to align flags (stylistic change only). * Drop stable tag, patch 01/02 doesn't apply earlier than 5.1. * Add tglx's suggested by tag for the tabs. * Carry Vaibhav's tested by tag from v3 since v4 is simply stylistic. Changes v2 -> v3: * Prefer $(CC_FLAGS_FTRACE) which is exported to -pg. * Also check CONFIG_STACKPROTECTOR and CONFIG_STACKPROTECTOR_STRONG. * Cc stable. Changes v1 -> v2: Rather than manually add -mno-sse, -mno-mmx, -mno-sse2, prefer to filter -pg flags. arch/x86/purgatory/Makefile | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) -- 2.22.0.709.g102302147b-goog