Skip to contentSkip to conversation

x86/retpoline: change RETPOLINE into CONFIG_RETPOLINE

PATCH In review Nadav Amit 2 participants 4 messages Dec 31, 2018 – Jan 3, 2019

Subsystems (4)
Stable branch x86 Module support x86 hardware vulnerabilities
lore
Share & tools
lore

Nadav Amit

author
#0 lore raw

PATCH 1/1 x86/retpoline: change RETPOLINE into CONFIG_RETPOLINE

A recent enhancement intentionally fails the kernel build if the compiler does not support retpolines and CONFIG_RETPOLINE is set.

However, the patch that introduced it did not change RETPOLINE macro references into CONFIG_RETPOLINE ones. As a result, indirect branches that are used by init functions are not kept (i.e., they use retpolines), and modules that do not use retpolines are marked as retpoline-safe.

Fix it be changing RETPOLINE into CONFIG_RETPOLINE.

Fixes 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler support")
Cc Peter Zijlstra <[email protected]>
Cc Zhenzhong Duan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc [email protected]
Signed-off-by Nadav Amit <[email protected]>
---
 arch/x86/kernel/cpu/bugs.c   | 2 +-
 include/linux/compiler-gcc.h | 2 +-
 include/linux/module.h       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -1
index 8654b8b0c848..1de0f4170178 100644
@@ -215,7 +215,7 @@ static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
 static enum spectre_v2_user_mitigation spectre_v2_user __ro_after_init =
 	SPECTRE_V2_USER_NONE;
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 static bool spectre_v2_bad_module;
 
 bool retpoline_module_ok(bool has_retpoline)
include/linux/compiler-gcc.h +1 -1
index 2010493e1040..977ddf2774f9 100644
@@ -68,7 +68,7 @@
  */
 #define uninitialized_var(x) x = x
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 #define __noretpoline __attribute__((__indirect_branch__("keep")))
 #endif
include/linux/module.h +1 -1
index fce6b4335e36..0c575f51fe57 100644
@@ -817,7 +817,7 @@ static inline void module_bug_finalize(const Elf_Ehdr *hdr,
 static inline void module_bug_cleanup(struct module *mod) {}
 #endif	/* CONFIG_GENERIC_BUG */
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 extern bool retpoline_module_ok(bool has_retpoline);
 #else
 static inline bool retpoline_module_ok(bool has_retpoline)
Signature · 1 line
2.17.1
#1 lore raw
Quoted 57 lines · Nadav Amit

A recent enhancement intentionally fails the kernel build if the compiler does not support retpolines and CONFIG_RETPOLINE is set.

However, the patch that introduced it did not change RETPOLINE macro references into CONFIG_RETPOLINE ones. As a result, indirect branches that are used by init functions are not kept (i.e., they use retpolines), and modules that do not use retpolines are marked as retpoline-safe.

Fix it be changing RETPOLINE into CONFIG_RETPOLINE.

Fixes 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler support")
Cc Peter Zijlstra <[email protected]>
Cc Zhenzhong Duan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc [email protected]
Signed-off-by Nadav Amit <[email protected]>
---
  arch/x86/kernel/cpu/bugs.c   | 2 +-
  include/linux/compiler-gcc.h | 2 +-
  include/linux/module.h       | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -1
index 8654b8b0c848..1de0f4170178 100644
@@ -215,7 +215,7 @@ static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
  static enum spectre_v2_user_mitigation spectre_v2_user __ro_after_init =
  	SPECTRE_V2_USER_NONE;
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  static bool spectre_v2_bad_module;
 
  bool retpoline_module_ok(bool has_retpoline)
include/linux/compiler-gcc.h +1 -1
index 2010493e1040..977ddf2774f9 100644
@@ -68,7 +68,7 @@
   */
  #define uninitialized_var(x) x = x
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  #define __noretpoline __attribute__((__indirect_branch__("keep")))
  #endif
include/linux/module.h +0 -0
index fce6b4335e36..0c575f51fe57 100644

include/linux/module.h static inline void module_bug_finalize(const Elf_Ehdr *hdr, · line 823

Quoted 8 lines
include/linux/module.h +1 -1
@@ -817,7 +817,7 @@ static inline void module_bug_finalize(const Elf_Ehdr *hdr,
  static inline void module_bug_cleanup(struct module *mod) {}
  #endif	/* CONFIG_GENERIC_BUG */
 
-#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  extern bool retpoline_module_ok(bool has_retpoline);
  #else
  static inline bool retpoline_module_ok(bool has_retpoline)

Someone sent a similar patch last month, see below link,  you missed the change in scripts/mod/modpost.c

https://lkml.org/lkml/2018/12/10/540

But you patch description is more clear.

Signature · 2 lines
Thanks
Zhenzhong
author
#2 lore raw
Quoted 58 lines · Zhenzhong Duan
Quoted 54 lines · Nadav Amit

A recent enhancement intentionally fails the kernel build if the compiler does not support retpolines and CONFIG_RETPOLINE is set.

However, the patch that introduced it did not change RETPOLINE macro references into CONFIG_RETPOLINE ones. As a result, indirect branches that are used by init functions are not kept (i.e., they use retpolines), and modules that do not use retpolines are marked as retpoline-safe.

Fix it be changing RETPOLINE into CONFIG_RETPOLINE.

Fixes 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler support")
Cc Peter Zijlstra <[email protected]>
Cc Zhenzhong Duan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc [email protected]
Signed-off-by Nadav Amit <[email protected]>
---
 arch/x86/kernel/cpu/bugs.c   | 2 +-
 include/linux/compiler-gcc.h | 2 +-
 include/linux/module.h       | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -0
index 8654b8b0c848..1de0f4170178 100644
@@ -215,7 +215,7 @@ static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
 static enum spectre_v2_user_mitigation spectre_v2_user __ro_after_init =
 	SPECTRE_V2_USER_NONE;
 -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 static bool spectre_v2_bad_module;
   bool retpoline_module_ok(bool has_retpoline)
include/linux/compiler-gcc.h +1 -0
index 2010493e1040..977ddf2774f9 100644
@@ -68,7 +68,7 @@
  */
 #define uninitialized_var(x) x = x
 -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 #define __noretpoline __attribute__((__indirect_branch__("keep")))
 #endif
 diff --git a/include/linux/module.h b/include/linux/module.h

index fce6b4335e36..0c575f51fe57 100644

include/linux/module.h +0 -0
@@ -817,7 +817,7 @@ static inline void module_bug_finalize(const Elf_Ehdr *hdr,
 static inline void module_bug_cleanup(struct module *mod) {}
Quoted 8 lines

include/linux/module.h static inline void module_bug_finalize(const Elf_Ehdr *hdr, · line 823

Quoted 6 lines
include/linux/module.h +1 -0
 #endif	/* CONFIG_GENERIC_BUG */
 -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
 extern bool retpoline_module_ok(bool has_retpoline);
 #else
 static inline bool retpoline_module_ok(bool has_retpoline)

Someone sent a similar patch last month, see below link, you missed the change in scripts/mod/modpost.c

Indeed. I missed this patch since it wasn’t on tip (and Google wasn’t helpful either).

I wonder why it has still not been applied.

#3 lore raw
Quoted 65 lines · Nadav Amit
Quoted 57 lines · Zhenzhong Duan
Quoted 53 lines · Nadav Amit

A recent enhancement intentionally fails the kernel build if the compiler does not support retpolines and CONFIG_RETPOLINE is set.

However, the patch that introduced it did not change RETPOLINE macro references into CONFIG_RETPOLINE ones. As a result, indirect branches that are used by init functions are not kept (i.e., they use retpolines), and modules that do not use retpolines are marked as retpoline-safe.

Fix it be changing RETPOLINE into CONFIG_RETPOLINE.

Fixes 4cd24de3a098 ("x86/retpoline: Make CONFIG_RETPOLINE depend on compiler support")
Cc Peter Zijlstra <[email protected]>
Cc Zhenzhong Duan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc David Woodhouse <[email protected]>
Cc Andy Lutomirski <[email protected]>
Cc Masahiro Yamada <[email protected]>
Cc [email protected]
Signed-off-by Nadav Amit <[email protected]>
---
  arch/x86/kernel/cpu/bugs.c   | 2 +-
  include/linux/compiler-gcc.h | 2 +-
  include/linux/module.h       | 2 +-
  3 files changed, 3 insertions(+), 3 deletions(-)
arch/x86/kernel/cpu/bugs.c +1 -0
index 8654b8b0c848..1de0f4170178 100644
@@ -215,7 +215,7 @@ static enum spectre_v2_mitigation spectre_v2_enabled __ro_after_init =
  static enum spectre_v2_user_mitigation spectre_v2_user __ro_after_init =
  	SPECTRE_V2_USER_NONE;
  -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  static bool spectre_v2_bad_module;
    bool retpoline_module_ok(bool has_retpoline)
include/linux/compiler-gcc.h +1 -0
index 2010493e1040..977ddf2774f9 100644
@@ -68,7 +68,7 @@
   */
  #define uninitialized_var(x) x = x
  -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  #define __noretpoline __attribute__((__indirect_branch__("keep")))
  #endif
  diff --git a/include/linux/module.h b/include/linux/module.h

index fce6b4335e36..0c575f51fe57 100644

include/linux/module.h +0 -0
@@ -817,7 +817,7 @@ static inline void module_bug_finalize(const Elf_Ehdr *hdr,
Quoted 8 lines

include/linux/module.h static inline void module_bug_finalize(const Elf_Ehdr *hdr, · line 823

Quoted 7 lines
include/linux/module.h +1 -0
  static inline void module_bug_cleanup(struct module *mod) {}
  #endif	/* CONFIG_GENERIC_BUG */
  -#ifdef RETPOLINE
+#ifdef CONFIG_RETPOLINE
  extern bool retpoline_module_ok(bool has_retpoline);
  #else
  static inline bool retpoline_module_ok(bool has_retpoline)

Someone sent a similar patch last month, see below link, you missed the change in scripts/mod/modpost.c

Quoted 4 lines

Indeed. I missed this patch since it wasn’t on tip (and Google wasn’t helpful either).

I wonder why it has still not been applied.

See https://lkml.org/lkml/2018/12/21/244

Signature · 2 lines
Thanks
Zhenzhong