From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755592AbbKCRmd (ORCPT ); Tue, 3 Nov 2015 12:42:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51255 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751415AbbKCRma (ORCPT ); Tue, 3 Nov 2015 12:42:30 -0500 Date: Tue, 3 Nov 2015 11:42:28 -0600 From: Josh Poimboeuf To: Miroslav Benes Cc: Seth Jennings , Jiri Kosina , Vojtech Pavlik , linux-kernel@vger.kernel.org, live-patching@vger.kernel.org, "Cyril B." Subject: [PATCH] livepatch: Cleanup page permission changes Message-ID: <20151103174228.GN27488@treble.redhat.com> References: <132b018cfe2c6e4cef4d1b62ac3ed70333734111.1446494413.git.jpoimboe@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org It's probably a good idea to keep the patches bisectable, so I made this a separate patch which applies on top of the first one. (Note that it completely removes all the code from the first patch, so there's no need for a v2 of the first patch which would have had Miroslav's suggested style changes.) ---8<--- Subject: [PATCH] livepatch: Cleanup page permission changes Calling set_memory_rw() and set_memory_ro() for every iteration of the loop in klp_write_object_relocations() is messy and inefficient. Change all the RO pages to RW before the loop and convert them back to RO after the loop. Suggested-by: Miroslav Benes Signed-off-by: Josh Poimboeuf --- arch/x86/kernel/livepatch.c | 25 ++----------------------- kernel/livepatch/core.c | 42 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c index d1d35cc..1062eff 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; + int 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/kernel/livepatch/core.c b/kernel/livepatch/core.c index 6e53441..328fbd5 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 @@ -131,6 +132,33 @@ static bool klp_initialized(void) return !!klp_root_kobj; } +#ifdef CONFIG_DEBUG_SET_MODULE_RONX +static void set_page_attributes(void *start, void *end, + int (*set)(unsigned long start, int num_pages)) +{ + unsigned long begin_pfn = PFN_DOWN((unsigned long)start); + unsigned long end_pfn = PFN_DOWN((unsigned long)end); + + if (end_pfn > begin_pfn) + set(begin_pfn << PAGE_SHIFT, end_pfn - begin_pfn); +} +static void set_module_ro_rw(struct module *mod) +{ + set_page_attributes(mod->module_core, + mod->module_core + mod->core_ro_size, + set_memory_rw); +} +static void set_module_ro_ro(struct module *mod) +{ + set_page_attributes(mod->module_core, + mod->module_core + mod->core_ro_size, + set_memory_ro); +} +#else +static void set_module_ro_rw(struct module *mod) {} +static void set_module_ro_ro(struct module *mod) {} +#endif + struct klp_find_arg { const char *objname; const char *name; @@ -283,7 +311,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 +320,14 @@ static int klp_write_object_relocations(struct module *pmod, if (WARN_ON(!obj->relocs)) return -EINVAL; + set_module_ro_rw(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 +339,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_ro_ro(pmod); + return ret; } static void notrace klp_ftrace_handler(unsigned long ip, -- 2.4.3