Re: [PATCH] Revert "module: Add retpoline tag to VERMAGIC"

[Jessica Yu]

+++ Andi Kleen [24/01/18 10:17 -0800]:
> On Wed, Jan 24, 2018 at 09:00:48AM -0800, Linus Torvalds wrote:
> > On Wed, Jan 24, 2018 at 6:28 AM, Greg Kroah-Hartman
> > <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > 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

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?

> Cc: jeyu@xxxxxxxxxx
> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
>
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 9c18da64daa9..ea707c91bd8c 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -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 */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index e4dc26185aa7..9064b20473a7 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -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))
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec825992..136ea6cabec6 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -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 "
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 98314b400a95..54deaa1066cf 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -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);