Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756546AbbKEVTs (ORCPT ); Thu, 5 Nov 2015 16:19:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40272 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756302AbbKEVTd (ORCPT ); Thu, 5 Nov 2015 16:19:33 -0500 From: Josh Poimboeuf To: Seth Jennings , Jiri Kosina , Vojtech Pavlik Cc: linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, "Cyril B." , Rusty Russell , Miroslav Benes Subject: [PATCH v2 3/3] livepatch: Cleanup module page permission changes Date: Thu, 5 Nov 2015 15:18:05 -0600 Message-Id: In-Reply-To: References: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5855 Lines: 186 Calling set_memory_rw() and set_memory_ro() for every iteration of the loop in klp_write_object_relocations() is messy, inefficient, and error-prone. Change all the read-only pages to read-write before the loop and convert them back to read-only again afterwards. The {un}set_module_core_ro_nx() functions are used to change the page permissions. Toggling NX isn't necessary in this case, but it's not highly performance sensitive code so it should be fine. Suggested-by: Miroslav Benes Signed-off-by: Josh Poimboeuf --- arch/x86/kernel/livepatch.c | 25 ++----------------------- include/linux/module.h | 4 ++++ kernel/livepatch/core.c | 15 ++++++++++----- kernel/module.c | 6 ++---- 4 files changed, 18 insertions(+), 32 deletions(-) diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c index d1d35cc..13eaf68 100644 --- a/arch/x86/kernel/livepatch.c +++ b/arch/x86/kernel/livepatch.c @@ -20,8 +20,6 @@ #include #include -#include -#include #include #include @@ -38,8 +36,7 @@ int klp_write_module_reloc(struct module *mod, unsigned long type, unsigned long loc, unsigned long value) { - int ret, numpages, size = 4; - bool readonly; + size_t size = 4; unsigned long val; unsigned long core = (unsigned long)mod->module_core; unsigned long core_size = mod->core_size; @@ -69,23 +66,5 @@ int klp_write_module_reloc(struct module *mod, unsigned long type, /* loc does not point to any symbol inside the module */ return -EINVAL; - readonly = false; - -#ifdef CONFIG_DEBUG_SET_MODULE_RONX - if (loc < core + mod->core_ro_size) - readonly = true; -#endif - - /* determine if the relocation spans a page boundary */ - numpages = ((loc & PAGE_MASK) == ((loc + size) & PAGE_MASK)) ? 1 : 2; - - if (readonly) - set_memory_rw(loc & PAGE_MASK, numpages); - - ret = probe_kernel_write((void *)loc, &val, size); - - if (readonly) - set_memory_ro(loc & PAGE_MASK, numpages); - - return ret; + return probe_kernel_write((void *)loc, &val, size); } diff --git a/include/linux/module.h b/include/linux/module.h index 3a19c79..557be36 100644 --- a/include/linux/module.h +++ b/include/linux/module.h @@ -766,9 +766,13 @@ extern int module_sysfs_initialized; #define __MODULE_STRING(x) __stringify(x) #ifdef CONFIG_DEBUG_SET_MODULE_RONX +extern void set_module_core_ro_nx(struct module *mod); +extern void unset_module_core_ro_nx(struct module *mod); extern void set_all_modules_text_rw(void); extern void set_all_modules_text_ro(void); #else +static inline void set_module_core_ro_nx(struct module *mod) { } +static inline void unset_module_core_ro_nx(struct module *mod) { } static inline void set_all_modules_text_rw(void) { } static inline void set_all_modules_text_ro(void) { } #endif diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 6e53441..1c94c4b 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -28,6 +28,7 @@ #include #include #include +#include /** * struct klp_ops - structure for tracking registered ftrace ops structs @@ -283,7 +284,7 @@ static int klp_find_external_symbol(struct module *pmod, const char *name, static int klp_write_object_relocations(struct module *pmod, struct klp_object *obj) { - int ret; + int ret = 0; struct klp_reloc *reloc; if (WARN_ON(!klp_is_object_loaded(obj))) @@ -292,12 +293,14 @@ static int klp_write_object_relocations(struct module *pmod, if (WARN_ON(!obj->relocs)) return -EINVAL; + unset_module_core_ro_nx(pmod); + for (reloc = obj->relocs; reloc->name; reloc++) { if (!klp_is_module(obj)) { ret = klp_verify_vmlinux_symbol(reloc->name, reloc->val); if (ret) - return ret; + goto out; } else { /* module, reloc->val needs to be discovered */ if (reloc->external) @@ -309,18 +312,20 @@ static int klp_write_object_relocations(struct module *pmod, reloc->name, &reloc->val); if (ret) - return ret; + goto out; } ret = klp_write_module_reloc(pmod, reloc->type, reloc->loc, reloc->val + reloc->addend); if (ret) { pr_err("relocation failed for symbol '%s' at 0x%016lx (%d)\n", reloc->name, reloc->val, ret); - return ret; + goto out; } } - return 0; +out: + set_module_core_ro_nx(pmod); + return ret; } static void notrace klp_ftrace_handler(unsigned long ip, diff --git a/kernel/module.c b/kernel/module.c index 14b2249..eb6a924 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1915,14 +1915,14 @@ static void set_section_ro_nx(void *base, } } -static void set_module_core_ro_nx(struct module *mod) +void set_module_core_ro_nx(struct module *mod) { set_section_ro_nx(mod->module_core, mod->core_text_size, mod->core_ro_size, mod->core_size, set_memory_ro, set_memory_nx); } -static void unset_module_core_ro_nx(struct module *mod) +void unset_module_core_ro_nx(struct module *mod) { set_section_ro_nx(mod->module_core, mod->core_text_size, mod->core_ro_size, mod->core_size, @@ -1989,9 +1989,7 @@ void set_all_modules_text_ro(void) mutex_unlock(&module_mutex); } #else -static void set_module_core_ro_nx(struct module *mod) { } static void set_module_init_ro_nx(struct module *mod) { } -static void unset_module_core_ro_nx(struct module *mod) { } static void unset_module_init_ro_nx(struct module *mod) { } #endif -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/