Since retpoline capable compilers are widely available, make
CONFIG_RETPOLINE hard depend on it.
Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
CONFIG_RETPOLINE which is used by kernel.
With all that stuff, the check of RETPOLINE is changed to
CONFIG_RETPOLINE.
This change is based on suggestion in https://lkml.org/lkml/2018/9/18/1016
Signed-off-by: Zhenzhong Duan <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Konrad Rzeszutek Wilk <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Masahiro Yamada <[email protected]>
Cc: Michal Marek <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/x86/Kconfig | 8 ++++----
arch/x86/Makefile | 5 +++--
arch/x86/entry/vdso/Makefile | 4 ++--
arch/x86/include/asm/nospec-branch.h | 10 ++++++----
arch/x86/kernel/cpu/bugs.c | 2 +-
arch/x86/kernel/vmlinux.lds.S | 2 +-
arch/x86/lib/Makefile | 2 +-
arch/x86/lib/retpoline.S | 2 ++
scripts/Makefile.build | 2 +-
10 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e129cd8..c26264e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4187,7 +4187,7 @@
Selecting 'on' will, and 'auto' may, choose a
mitigation method at run time according to the
CPU, the available microcode, the setting of the
- CONFIG_RETPOLINE configuration option, and the
+ CONFIG_RETPOLINE_SUPPORT configuration option, and the
compiler with which the kernel was built.
Specific mitigations can also be selected manually:
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cbd5f28..766563f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -433,7 +433,7 @@ config GOLDFISH
def_bool y
depends on X86_GOLDFISH
-config RETPOLINE
+config RETPOLINE_SUPPORT
bool "Avoid speculative indirect branches in kernel"
default y
select STACK_VALIDATION if HAVE_STACK_VALIDATION
@@ -443,9 +443,9 @@ config RETPOLINE
branches. Requires a compiler with -mindirect-branch=thunk-extern
support for full protection. The kernel may run slower.
- Without compiler support, at least indirect branches in assembler
- code are eliminated. Since this includes the syscall entry path,
- it is not entirely pointless.
+ Since retpoline capable compilers are widely available, kernel doesn't
+ use CONFIG_RETPOLINE_SUPPORT directly but use CONFIG_RETPOLINE which
+ is enabled when compiler support retpoline.
config INTEL_RDT
bool "Intel Resource Director Technology support"
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 5b562e4..7ed35b1 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -221,9 +221,10 @@ KBUILD_CFLAGS += -Wno-sign-compare
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
# Avoid indirect branches in kernel to deal with Spectre
-ifdef CONFIG_RETPOLINE
+ifdef CONFIG_RETPOLINE_SUPPORT
ifneq ($(RETPOLINE_CFLAGS),)
- KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DRETPOLINE
+ KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DCONFIG_RETPOLINE
+ KBUILD_AFLAGS += -DCONFIG_RETPOLINE
endif
endif
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 141d415..87acde1 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -70,7 +70,7 @@ CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \
-fno-omit-frame-pointer -foptimize-sibling-calls \
-DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
-ifdef CONFIG_RETPOLINE
+ifdef CONFIG_RETPOLINE_SUPPORT
ifneq ($(RETPOLINE_VDSO_CFLAGS),)
CFL += $(RETPOLINE_VDSO_CFLAGS)
endif
@@ -145,7 +145,7 @@ KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
KBUILD_CFLAGS_32 += -fno-omit-frame-pointer
KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING
-ifdef CONFIG_RETPOLINE
+ifdef CONFIG_RETPOLINE_SUPPORT
ifneq ($(RETPOLINE_VDSO_CFLAGS),)
KBUILD_CFLAGS_32 += $(RETPOLINE_VDSO_CFLAGS)
endif
diff --git a/arch/x86/include/asm/nospec-branch.h b/arch/x86/include/asm/nospec-branch.h
index 80dc144..8b09cbb 100644
--- a/arch/x86/include/asm/nospec-branch.h
+++ b/arch/x86/include/asm/nospec-branch.h
@@ -162,11 +162,12 @@
_ASM_PTR " 999b\n\t" \
".popsection\n\t"
-#if defined(CONFIG_X86_64) && defined(RETPOLINE)
+#ifdef CONFIG_RETPOLINE
+#ifdef CONFIG_X86_64
/*
- * Since the inline asm uses the %V modifier which is only in newer GCC,
- * the 64-bit one is dependent on RETPOLINE not CONFIG_RETPOLINE.
+ * Inline asm uses the %V modifier which is only in newer GCC
+ * which is ensured when CONFIG_RETPOLINE is defined.
*/
# define CALL_NOSPEC \
ANNOTATE_NOSPEC_ALTERNATIVE \
@@ -181,7 +182,7 @@
X86_FEATURE_RETPOLINE_AMD)
# define THUNK_TARGET(addr) [thunk_target] "r" (addr)
-#elif defined(CONFIG_X86_32) && defined(CONFIG_RETPOLINE)
+#else /* CONFIG_X86_32 */
/*
* For i386 we use the original ret-equivalent retpoline, because
* otherwise we'll run out of registers. We don't care about CET
@@ -211,6 +212,7 @@
X86_FEATURE_RETPOLINE_AMD)
# define THUNK_TARGET(addr) [thunk_target] "rm" (addr)
+#endif
#else /* No retpoline for C / inline asm */
# define CALL_NOSPEC "call *%[thunk_target]\n"
# define THUNK_TARGET(addr) [thunk_target] "rm" (addr)
diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
index c37e66e..d0108fb 100644
--- a/arch/x86/kernel/cpu/bugs.c
+++ b/arch/x86/kernel/cpu/bugs.c
@@ -252,7 +252,7 @@ static void __init spec2_print_if_secure(const char *reason)
static inline bool retp_compiler(void)
{
- return __is_defined(RETPOLINE);
+ return __is_defined(CONFIG_RETPOLINE);
}
static inline bool match_option(const char *arg, int arglen, const char *opt)
diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
index 0d618ee..07533f7 100644
--- a/arch/x86/kernel/vmlinux.lds.S
+++ b/arch/x86/kernel/vmlinux.lds.S
@@ -136,7 +136,7 @@ SECTIONS
*(.fixup)
*(.gnu.warning)
-#ifdef CONFIG_RETPOLINE
+#ifdef CONFIG_RETPOLINE_SUPPORT
__indirect_thunk_start = .;
*(.text.__x86.indirect_thunk)
__indirect_thunk_end = .;
diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile
index 25a972c..edfa7ab 100644
--- a/arch/x86/lib/Makefile
+++ b/arch/x86/lib/Makefile
@@ -27,7 +27,7 @@ lib-$(CONFIG_RWSEM_XCHGADD_ALGORITHM) += rwsem.o
lib-$(CONFIG_INSTRUCTION_DECODER) += insn.o inat.o insn-eval.o
lib-$(CONFIG_RANDOMIZE_BASE) += kaslr.o
lib-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o
-lib-$(CONFIG_RETPOLINE) += retpoline.o
+lib-$(CONFIG_RETPOLINE_SUPPORT) += retpoline.o
obj-y += msr.o msr-reg.o msr-reg-export.o hweight.o
diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
index c909961..81366ad 100644
--- a/arch/x86/lib/retpoline.S
+++ b/arch/x86/lib/retpoline.S
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#ifdef CONFIG_RETPOLINE
#include <linux/stringify.h>
#include <linux/linkage.h>
#include <asm/dwarf2.h>
@@ -46,3 +47,4 @@ GENERATE_THUNK(r13)
GENERATE_THUNK(r14)
GENERATE_THUNK(r15)
#endif
+#endif
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 54da4b0..259d9c6 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -247,7 +247,7 @@ endif
ifdef CONFIG_GCOV_KERNEL
objtool_args += --no-unreachable
endif
-ifdef CONFIG_RETPOLINE
+ifdef CONFIG_RETPOLINE_SUPPORT
ifneq ($(RETPOLINE_CFLAGS),)
objtool_args += --retpoline
endif
--
1.8.3.1
Hi,
On Tue, Oct 30, 2018 at 3:57 PM Zhenzhong Duan
<[email protected]> wrote:
>
> Since retpoline capable compilers are widely available, make
> CONFIG_RETPOLINE hard depend on it.
>
> Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
> CONFIG_RETPOLINE which is used by kernel.
>
> With all that stuff, the check of RETPOLINE is changed to
> CONFIG_RETPOLINE.
>
> This change is based on suggestion in https://lkml.org/lkml/2018/9/18/1016
>
> Signed-off-by: Zhenzhong Duan <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: H. Peter Anvin <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Konrad Rzeszutek Wilk <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Masahiro Yamada <[email protected]>
> Cc: Michal Marek <[email protected]>
> ---
Instead of adding another CONFIG option,
does it make sense to add compiler support checks
to 'depends on' syntax ?
config RETPOLINE
bool "Avoid speculative indirect branches in kernel"
depends on $(cc-option,-mindirect-branch=thunk-extern
-mindirect-branch-register) || \
$(cc-option,-mretpoline-external-thunk)
default y
select STACK_VALIDATION if HAVE_STACK_VALIDATION
--
Best Regards
Masahiro Yamada
On Tue, Oct 30, 2018 at 06:39:24PM +0900, Masahiro Yamada wrote:
> Hi,
>
>
>
> On Tue, Oct 30, 2018 at 3:57 PM Zhenzhong Duan
> <[email protected]> wrote:
> >
> > Since retpoline capable compilers are widely available, make
> > CONFIG_RETPOLINE hard depend on it.
> >
> > Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
> > CONFIG_RETPOLINE which is used by kernel.
> >
> > With all that stuff, the check of RETPOLINE is changed to
> > CONFIG_RETPOLINE.
> >
> > This change is based on suggestion in https://lkml.org/lkml/2018/9/18/1016
> >
> > Signed-off-by: Zhenzhong Duan <[email protected]>
> > Cc: Thomas Gleixner <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Borislav Petkov <[email protected]>
> > Cc: Daniel Borkmann <[email protected]>
> > Cc: David Woodhouse <[email protected]>
> > Cc: H. Peter Anvin <[email protected]>
> > Cc: Ingo Molnar <[email protected]>
> > Cc: Konrad Rzeszutek Wilk <[email protected]>
> > Cc: Andy Lutomirski <[email protected]>
> > Cc: Masahiro Yamada <[email protected]>
> > Cc: Michal Marek <[email protected]>
> > ---
>
>
> Instead of adding another CONFIG option,
> does it make sense to add compiler support checks
> to 'depends on' syntax ?
>
>
> config RETPOLINE
> bool "Avoid speculative indirect branches in kernel"
> depends on $(cc-option,-mindirect-branch=thunk-extern
> -mindirect-branch-register) || \
> $(cc-option,-mretpoline-external-thunk)
> default y
> select STACK_VALIDATION if HAVE_STACK_VALIDATION
That seems to be what we did for stackprotector, which is similar in
that it used to fail the build. So yes, this seems sane.
On Mon, Oct 29, 2018 at 11:55:04PM -0700, Zhenzhong Duan wrote:
> Since retpoline capable compilers are widely available, make
> CONFIG_RETPOLINE hard depend on it.
>
> Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
> CONFIG_RETPOLINE which is used by kernel.
>
> With all that stuff, the check of RETPOLINE is changed to
> CONFIG_RETPOLINE.
So what happens when we select CONFIG_RETPOLINE but do not have
RETPOLINE_SUPPORT ? From a quick reading we'll silently build a
!retpoline kernel. I would expect a build failure.
On 2018/10/30 16:32, Peter Zijlstra wrote:
> On Mon, Oct 29, 2018 at 11:55:04PM -0700, Zhenzhong Duan wrote:
>> Since retpoline capable compilers are widely available, make
>> CONFIG_RETPOLINE hard depend on it.
>>
>> Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
>> CONFIG_RETPOLINE which is used by kernel.
>>
>> With all that stuff, the check of RETPOLINE is changed to
>> CONFIG_RETPOLINE.
>
> So what happens when we select CONFIG_RETPOLINE but do not have
> RETPOLINE_SUPPORT ? From a quick reading we'll silently build a
> !retpoline kernel. I would expect a build failure.
CONFIG_RETPOLINE is only defined when CONFIG_RETPOLINE_SUPPORT is
selected. See below chunk.
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -221,9 +221,10 @@ KBUILD_CFLAGS += -Wno-sign-compare
KBUILD_CFLAGS += -fno-asynchronous-unwind-tables
# Avoid indirect branches in kernel to deal with Spectre
-ifdef CONFIG_RETPOLINE
+ifdef CONFIG_RETPOLINE_SUPPORT
ifneq ($(RETPOLINE_CFLAGS),)
- KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DRETPOLINE
+ KBUILD_CFLAGS += $(RETPOLINE_CFLAGS) -DCONFIG_RETPOLINE
+ KBUILD_AFLAGS += -DCONFIG_RETPOLINE
endif
endif
Thanks
Zhenzhong
On 2018/10/30 18:09, Peter Zijlstra wrote:
> On Tue, Oct 30, 2018 at 06:39:24PM +0900, Masahiro Yamada wrote:
>> Hi,
>>
>>
>>
>> On Tue, Oct 30, 2018 at 3:57 PM Zhenzhong Duan
>> <[email protected]> wrote:
>>>
>>> Since retpoline capable compilers are widely available, make
>>> CONFIG_RETPOLINE hard depend on it.
>>>
>>> Change KBUILD to use CONFIG_RETPOLINE_SUPPORT to avoid conflict with
>>> CONFIG_RETPOLINE which is used by kernel.
>>>
>>> With all that stuff, the check of RETPOLINE is changed to
>>> CONFIG_RETPOLINE.
>>>
>>> This change is based on suggestion in https://lkml.org/lkml/2018/9/18/1016
>>>
>>> Signed-off-by: Zhenzhong Duan <[email protected]>
>>> Cc: Thomas Gleixner <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Borislav Petkov <[email protected]>
>>> Cc: Daniel Borkmann <[email protected]>
>>> Cc: David Woodhouse <[email protected]>
>>> Cc: H. Peter Anvin <[email protected]>
>>> Cc: Ingo Molnar <[email protected]>
>>> Cc: Konrad Rzeszutek Wilk <[email protected]>
>>> Cc: Andy Lutomirski <[email protected]>
>>> Cc: Masahiro Yamada <[email protected]>
>>> Cc: Michal Marek <[email protected]>
>>> ---
>>
>>
>> Instead of adding another CONFIG option,
>> does it make sense to add compiler support checks
>> to 'depends on' syntax ?
>>
>>
>> config RETPOLINE
>> bool "Avoid speculative indirect branches in kernel"
>> depends on $(cc-option,-mindirect-branch=thunk-extern
>> -mindirect-branch-register) || \
>> $(cc-option,-mretpoline-external-thunk)
>> default y
>> select STACK_VALIDATION if HAVE_STACK_VALIDATION
Looks better, thanks for suggestion.
>
> That seems to be what we did for stackprotector, which is similar in
> that it used to fail the build. So yes, this seems sane.
Should I add a scripts/gcc-x86_64-has-retpoline.sh like what
stackprotector does as below or there is a simpler way?
config CC_HAS_SANE_STACKPROTECTOR
bool
default
$(success,$(srctree)/scripts/gcc-x86_64-has-stack-protector.sh $(CC)) if
64BIT
default
$(success,$(srctree)/scripts/gcc-x86_32-has-stack-protector.sh $(CC))
help
We have to make sure stack protector is unconditionally
disabled if
the compiler produces broken code.
Thanks
Zhenzhong