Revert "module: Add retpoline tag to VERMAGIC"
Subsystems (3)
PATCH 1/1 Revert "module: Add retpoline tag to VERMAGIC"
This reverts commit 6cfb521ac0d5b97470883ff9b7facae264b7ab12.
Turns out distros do not want to make retpoline as part of their "ABI", so this patch should not have been merged. Sorry Andi, this was my fault, I suggested it when your original patch was the "correct" way of doing this instead.
---
Linus, if there are no objections, can you apply this revert to your tree now so this doesn't get into 4.15? It's already in the stable releases but Jiri pointed out he instantly reverted it for SuSE's kernels as it does not do what they want it to do. They want to just taint the kernel, not prevent modules from being loaded entirely.
include/linux/vermagic.h | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
include/linux/vermagic.h +1 -7
@@ -31,17 +31,11 @@#else #define MODULE_RANDSTRUCT_PLUGIN #endif -#ifdef RETPOLINE -#define MODULE_VERMAGIC_RETPOLINE "retpoline " -#else -#define MODULE_VERMAGIC_RETPOLINE "" -#endif #define VERMAGIC_STRING \ UTS_RELEASE " " \ MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT \ MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS \ MODULE_ARCH_VERMAGIC \ - MODULE_RANDSTRUCT_PLUGIN \ - MODULE_VERMAGIC_RETPOLINE + MODULE_RANDSTRUCT_PLUGIN
Signature · 1 line
2.16.1
Quoted 6 lines · Greg Kroah-Hartman
This reverts commit
6cfb521ac0d5b97470883ff9b7facae264b7ab12.Turns out distros do not want to make retpoline as part of their "ABI", so this patch should not have been merged. Sorry Andi, this was my fault, I suggested it when your original patch was the "correct" way of doing this instead.
It's ok for me to revert, but if it's reverted should consider the warning only patch version instead.
I had posted that earlier, but reviewers in the end preferred the simple modversions variant.
I think we should have something at least for 4.15 to catch mistakes.
-Andi
Linus Torvalds
AppliedQuoted 3 lines · Greg Kroah-Hartman
Linus, if there are no objections, can you apply this revert to your tree now so this doesn't get into 4.15?
Applied.
Linus
Quoted 7 lines · Linus Torvalds
Quoted 3 lines · Greg Kroah-Hartman
Linus, if there are no objections, can you apply this revert to your tree now so this doesn't get into 4.15?
Applied.
So can we get the warning replacement? It would be good to have some kind of solution.
-Andi
----
retpoline/module: Warn for missing retpoline in module
There's a risk that a kernel that has full retpoline mitigations becomes vulnerable when a module gets loaded that hasn't been compiled with the right compiler or the right option.
We cannot fix it, but should at least warn the user when that happens.
When the a module hasn't been compiled with a retpoline aware compiler, print a warning and change the SPECTRE_V2 mitigation mode to show the system is vulnerable now.
For modules it is checked at compile time, however it cannot check assembler or other non compiled objects used in the module link.
v2: Change warning message
v3: Port to latest tree
v4: Remove tainting
arch/x86/include/asm/processor.h +4 -0
@@ -970,4 +970,8 @@ bool xen_set_default_idle(void);void stop_this_cpu (void *dummy);void df_debug (struct pt_regs *regs,long error_code); + +void disable_retpoline (void ); +bool retpoline_enabled (void ); +#endif /* _ASM_X86_PROCESSOR_H */
arch/x86/kernel/cpu/bugs.c +12 -0
@@ -93,6 +93,18 @@ static const char *spectre_v2_strings[] = {static enum spectre_v2_mitigation spectre_v2_enabled =SPECTRE_V2_NONE ; +/* A module has been loaded. Disable reporting that we're good. */ +void disable_retpoline (void ) +{ + spectre_v2_enabled =SPECTRE_V2_NONE ; +pr_err ("system may be vunerable to spectre\n" ); +} + +bool retpoline_enabled (void ) +{ +return spectre_v2_enabled !=SPECTRE_V2_NONE ; +} +static void __init spec2_print_if_insecure (const char *reason) {if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 ))
kernel/module.c +7 -1
@@ -3020,7 +3020,13 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) mod->name);add_taint_module (mod,TAINT_OOT_MODULE ,LOCKDEP_STILL_OK ); } - +#ifdef RETPOLINE +if (retpoline_enabled () && !get_modinfo (info,"retpoline" )) { +pr_warn ("%s: loading module not compiled with retpoline compiler.\n" , + mod->name); +disable_retpoline (); + } +#endif if (get_modinfo (info,"staging" )) {add_taint_module (mod,TAINT_CRAP ,LOCKDEP_STILL_OK ); pr_warn("%s: module is from the staging directory, the quality "
scripts/mod/modpost.c +9 -0
@@ -2165,6 +2165,14 @@ static void add_intree_flag(struct buffer *b, int is_intree)buf_printf (b,"\nMODULE_INFO(intree, \"Y\");\n" ); } +/* Cannot check for assembler */ +static void add_retpoline (struct buffer *b) +{ +buf_printf (b,"\n#ifdef RETPOLINE\n" ); +buf_printf (b,"MODULE_INFO(retpoline, \"Y\");\n" ); +buf_printf (b,"#endif\n" ); +} +static void add_staging_flag (struct buffer *b,const char *name) {static const char *staging_dir ="drivers/staging" ;
@@ -2506,6 +2514,7 @@ int main(int argc, char **argv) err |=check_modname_len (mod);add_header (&buf, mod);add_intree_flag (&buf, !external_module); +add_retpoline (&buf);add_staging_flag (&buf, mod->name); err |=add_versions (&buf, mod);add_depends (&buf, mod, modules);
arch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 100
Quoted 13 lines · Andi Kleen
arch/x86/kernel/cpu/bugs.c +5
-0@@ -93,6 +93,18 @@ static const char *spectre_v2_strings[] = {static enum spectre_v2_mitigation spectre_v2_enabled =SPECTRE_V2_NONE ; +/* A module has been loaded. Disable reporting that we're good. */ +void disable_retpoline (void ) +{ + spectre_v2_enabled =SPECTRE_V2_NONE ; +pr_err ("system may be vunerable to spectre\n" );
vulnerable
Quoted 1 line
arch/x86/kernel/cpu/bugs.c +1
-0+}
Signature · 1 line
~Randy
arch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 99
Quoted 4 lines · Andi Kleen
arch/x86/kernel/cpu/bugs.c +4
-0+/* A module has been loaded. Disable reporting that we're good. */ +void disable_retpoline (void ) +{ + spectre_v2_enabled = SPECTRE_V2_NONE;
That wants to be one of the 'MINIMAL' variants.
arch/x86/kernel/cpu/bugs.c static const char *spectre_v2_strings[] = { · line 105
Quoted 6 lines
+ pr_err("system may be vunerable to spectre\n");
arch/x86/kernel/cpu/bugs.c +5
-0+} + +bool retpoline_enabled (void ) +{ + return spectre_v2_enabled != SPECTRE_V2_NONE;
How about checking for X86_FEATURE_RETPOLINE?
Quoted 11 lines · Andi Kleen
Quoted 7 lines · Linus Torvalds
Quoted 3 lines · Greg Kroah-Hartman
Linus, if there are no objections, can you apply this revert to your tree now so this doesn't get into 4.15?
Applied.
So can we get the warning replacement? It would be good to have some kind of solution.
I already reviewed that and had comments. So, no. This one is not going to go anywhere.
Thanks,
tglx
+++ Andi Kleen [24/01/18 10:17 -0800]:
Quoted 29 lines · Linus Torvalds
Quoted 7 lines
Quoted 3 lines · Greg Kroah-Hartman
Linus, if there are no objections, can you apply this revert to your tree now so this doesn't get into 4.15?
Applied.
So can we get the warning replacement? It would be good to have some kind of solution.
-Andi
----
retpoline/module: Warn for missing retpoline in module
There's a risk that a kernel that has full retpoline mitigations becomes vulnerable when a module gets loaded that hasn't been compiled with the right compiler or the right option.
We cannot fix it, but should at least warn the user when that happens.
When the a module hasn't been compiled with a retpoline aware compiler, print a warning and change the
SPECTRE_V2mitigation mode to show the system is vulnerable now.
Quoted 6 lines
For modules it is checked at compile time, however it cannot check assembler or other non compiled objects used in the module link.
v2: Change warning message
v3: Port to latest tree
v4: Remove tainting
So I thought distros wanted the module taint after all, as Greg mentioned, or is that still overkill? Would the printed warning be sufficient for the distro folks?
Quoted 87 lines
Signed-off-by Andi Kleen <[email protected]>Signed-off-by David Woodhouse <[email protected]>arch/x86/include/asm/processor.h +4
-0@@ -970,4 +970,8 @@ bool xen_set_default_idle(void);void stop_this_cpu (void *dummy);void df_debug (struct pt_regs *regs,long error_code); + +void disable_retpoline (void ); +bool retpoline_enabled (void ); +#endif /* _ASM_X86_PROCESSOR_H */ arch/x86/kernel/cpu/bugs.c +12
-0@@ -93,6 +93,18 @@ static const char *spectre_v2_strings[] = {static enum spectre_v2_mitigation spectre_v2_enabled =SPECTRE_V2_NONE ; +/* A module has been loaded. Disable reporting that we're good. */ +void disable_retpoline (void ) +{ + spectre_v2_enabled =SPECTRE_V2_NONE ; +pr_err ("system may be vunerable to spectre\n" ); +} + +bool retpoline_enabled (void ) +{ +return spectre_v2_enabled !=SPECTRE_V2_NONE ; +} +static void __init spec2_print_if_insecure (const char *reason) {if (boot_cpu_has_bug (X86_BUG_SPECTRE_V2 ))kernel/module.c +7
-1@@ -3020,7 +3020,13 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags) mod->name);add_taint_module (mod,TAINT_OOT_MODULE ,LOCKDEP_STILL_OK ); } - +#ifdef RETPOLINE +if (retpoline_enabled () && !get_modinfo (info,"retpoline" )) { +pr_warn ("%s: loading module not compiled with retpoline compiler.\n" , + mod->name); +disable_retpoline (); + } +#endif if (get_modinfo (info,"staging" )) {add_taint_module (mod,TAINT_CRAP ,LOCKDEP_STILL_OK ); pr_warn("%s: module is from the staging directory, the quality "scripts/mod/modpost.c +9
-0@@ -2165,6 +2165,14 @@ static void add_intree_flag(struct buffer *b, int is_intree)buf_printf (b,"\nMODULE_INFO(intree, \"Y\");\n" ); } +/* Cannot check for assembler */ +static void add_retpoline (struct buffer *b) +{ +buf_printf (b,"\n#ifdef RETPOLINE\n" ); +buf_printf (b,"MODULE_INFO(retpoline, \"Y\");\n" ); +buf_printf (b,"#endif\n" ); +} +static void add_staging_flag (struct buffer *b,const char *name) {static const char *staging_dir ="drivers/staging" ;@@ -2506,6 +2514,7 @@ int main(int argc, char **argv) err |=check_modname_len (mod);add_header (&buf, mod);add_intree_flag (&buf, !external_module); +add_retpoline (&buf);add_staging_flag (&buf, mod->name); err |=add_versions (&buf, mod);add_depends (&buf, mod, modules);