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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20180126001958.vcqsxcvwfrvdmsf2@redbean.localdomain>
Date:   Fri, 26 Jan 2018 01:20:00 +0100
From:   Jessica Yu <jeyu@...nel.org>
To:     Andi Kleen <ak@...ux.intel.com>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Kosina <jikos@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Rusty Russell <rusty@...tcorp.com.au>,
        "Van De Ven, Arjan" <arjan.van.de.ven@...el.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Revert "module: Add retpoline tag to VERMAGIC"

+++ 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@...uxfoundation.org> 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@...nel.org
>Signed-off-by: Andi Kleen <ak@...ux.intel.com>
>Signed-off-by: David Woodhouse <dwmw@...zon.co.uk>
>
>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);
>