From patchwork Wed Dec 5 01:34:07 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nadav Amit X-Patchwork-Id: 10713331 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A7EF21731 for ; Wed, 5 Dec 2018 08:52:39 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 98BD22CCD2 for ; Wed, 5 Dec 2018 08:52:39 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 96FF82CCDC; Wed, 5 Dec 2018 08:52:39 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.4 required=2.0 tests=BAYES_00,DATE_IN_PAST_06_12, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 337CE2CCD2 for ; Wed, 5 Dec 2018 08:52:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727545AbeLEIwc (ORCPT ); Wed, 5 Dec 2018 03:52:32 -0500 Received: from ex13-edg-ou-001.vmware.com ([208.91.0.189]:33316 "EHLO EX13-EDG-OU-001.vmware.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727337AbeLEIwN (ORCPT ); Wed, 5 Dec 2018 03:52:13 -0500 Received: from sc9-mailhost3.vmware.com (10.113.161.73) by EX13-EDG-OU-001.vmware.com (10.113.208.155) with Microsoft SMTP Server id 15.0.1156.6; Wed, 5 Dec 2018 00:52:03 -0800 Received: from sc2-haas01-esx0118.eng.vmware.com (sc2-haas01-esx0118.eng.vmware.com [10.172.44.118]) by sc9-mailhost3.vmware.com (Postfix) with ESMTP id 3EDB341392; Wed, 5 Dec 2018 00:52:05 -0800 (PST) From: Nadav Amit To: Ingo Molnar CC: , , "H. Peter Anvin" , Thomas Gleixner , Borislav Petkov , Andy Lutomirski , Nadav Amit , Dave Hansen , Peter Zijlstra , , , , Nadav Amit , Rick P Edgecombe , Will Deacon Subject: [PATCH v7 13/14] module: Do not set nx for module memory before freeing Date: Tue, 4 Dec 2018 17:34:07 -0800 Message-ID: <20181205013408.47725-14-namit@vmware.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181205013408.47725-1-namit@vmware.com> References: <20181205013408.47725-1-namit@vmware.com> MIME-Version: 1.0 Received-SPF: None (EX13-EDG-OU-001.vmware.com: namit@vmware.com does not designate permitted sender hosts) Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP When module memory is about to be freed, there is no apparent reason to make it (and its data) executable, but that's exactly what is done today. This is not efficient and not secure. There are various theories why it was done, but none of them seem as something that really require it today. nios2 uses kmalloc for module memory, but anyhow it does not change the PTEs of the module memory. In x86, changing vmalloc'd memory mappings also modifies the direct mapping alias, but the NX-bit is not modified in such way. So let's remove it. Andy suggested that the changes of the PTEs can be avoided (excluding the direct-mapping alias), which is true. However, in x86 it requires some cleanup of the contiguous page allocator, which is outside of the scope of this patch-set. Cc: Rick P Edgecombe Cc: Will Deacon Cc: Andy Lutomirski Signed-off-by: Nadav Amit --- kernel/module.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/kernel/module.c b/kernel/module.c index 7cb207249437..57c5b23746e7 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -2027,20 +2027,29 @@ void set_all_modules_text_ro(void) mutex_unlock(&module_mutex); } -static void disable_ro_nx(const struct module_layout *layout) +static void module_restore_mappings(const struct module_layout *layout) { - if (rodata_enabled) { - frob_text(layout, set_memory_rw); - frob_rodata(layout, set_memory_rw); - frob_ro_after_init(layout, set_memory_rw); - } - frob_rodata(layout, set_memory_x); - frob_ro_after_init(layout, set_memory_x); - frob_writable_data(layout, set_memory_x); + /* + * First, make the mappings of the code non-executable to prevent + * transient W+X mappings from being set when the text is set as RW. + */ + frob_text(layout, set_memory_nx); + + if (!rodata_enabled) + return; + + /* + * Second, set the memory as writable. Although the module memory is + * about to be freed, these calls are required (at least on x86) to + * restore the direct map to its "correct" state. + */ + frob_text(layout, set_memory_rw); + frob_rodata(layout, set_memory_rw); + frob_ro_after_init(layout, set_memory_rw); } #else -static void disable_ro_nx(const struct module_layout *layout) { } +static void module_restore_mappings(const struct module_layout *layout) { } static void module_enable_nx(const struct module *mod) { } static void module_disable_nx(const struct module *mod) { } #endif @@ -2173,7 +2182,7 @@ static void free_module(struct module *mod) mutex_unlock(&module_mutex); /* This may be empty, but that's OK */ - disable_ro_nx(&mod->init_layout); + module_restore_mappings(&mod->init_layout); module_arch_freeing_init(mod); module_memfree(mod->init_layout.base); kfree(mod->args); @@ -2183,7 +2192,7 @@ static void free_module(struct module *mod) lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size); /* Finally, free the core (containing the module structure) */ - disable_ro_nx(&mod->core_layout); + module_restore_mappings(&mod->core_layout); module_memfree(mod->core_layout.base); } @@ -3507,7 +3516,7 @@ static noinline int do_init_module(struct module *mod) #endif module_enable_ro(mod, true); mod_tree_remove_init(mod); - disable_ro_nx(&mod->init_layout); + module_restore_mappings(&mod->init_layout); module_arch_freeing_init(mod); mod->init_layout.base = NULL; mod->init_layout.size = 0;