+++ Petr Mladek [18/08/16 11:51 +0200]:
On Wed 2016-08-17 20:58:29, Jessica Yu wrote:Implement arch_klp_init_object_loaded() for x86, which applies alternatives/paravirt patches. This fixes the order in which relocations and alternatives/paravirt patches are applied. Previously, if a patch module had alternatives or paravirt patches, these were applied first by the module loader before livepatch can apply per-object relocations. The (buggy) sequence of events was: (1) Load patch module (2) Apply alternatives and paravirt patches to patch module * Note that these are applied to the new functions in the patch module (3) Apply per-object relocations to patch module when target module loads. * This clobbers what was written in step 2 This lead to crashes and corruption in general, since livepatch would overwrite or step on previously applied alternative/paravirt patches. The correct sequence of events should be: (1) Load patch module (2) Apply per-object relocations to patch module (3) Apply alternatives and paravirt patches to patch module This is fixed by delaying paravirt/alternatives patching until after relocations are applied. Any .altinstructions or .parainstructions sections are prefixed with ".klp.arch.${objname}" and applied in arch_klp_init_object_loaded(). Signed-off-by: Jessica Yu <jeyu@xxxxxxxxxx> --- arch/x86/kernel/Makefile | 1 + arch/x86/kernel/livepatch.c | 65 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 arch/x86/kernel/livepatch.c diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile index d3f49c3..92fd50c 100644 --- a/arch/x86/kernel/Makefile +++ b/arch/x86/kernel/Makefile @@ -81,6 +81,7 @@ obj-$(CONFIG_X86_MPPARSE) += mpparse.o obj-y += apic/ obj-$(CONFIG_X86_REBOOTFIXUPS) += reboot_fixups_32.o obj-$(CONFIG_DYNAMIC_FTRACE) += ftrace.o +obj-$(CONFIG_LIVEPATCH) += livepatch.o obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o obj-$(CONFIG_FTRACE_SYSCALLS) += ftrace.o obj-$(CONFIG_X86_TSC) += trace_clock.o diff --git a/arch/x86/kernel/livepatch.c b/arch/x86/kernel/livepatch.c new file mode 100644 index 0000000..e9d252d --- /dev/null +++ b/arch/x86/kernel/livepatch.c @@ -0,0 +1,65 @@ +/* + * livepatch.c - x86-specific Kernel Live Patching Core + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/module.h> +#include <linux/kallsyms.h> +#include <linux/livepatch.h> +#include <asm/text-patching.h> + +/* Apply per-object alternatives. Based on x86 module_finalize() */ +void arch_klp_init_object_loaded(struct klp_patch *patch, + struct klp_object *obj) +{ + int cnt; + struct klp_modinfo *info; + Elf_Shdr *s, *alt = NULL, *para = NULL; + void *aseg, *pseg; + const char *objname; + char sec_objname[MODULE_NAME_LEN]; + char secname[KSYM_NAME_LEN]; + + info = patch->mod->klp_info; + objname = obj->name ? obj->name : "vmlinux"; + + /* See livepatch core code for BUILD_BUG_ON() explanation */ + BUILD_BUG_ON(MODULE_NAME_LEN < 56 || KSYM_NAME_LEN != 128); + + for (s = info->sechdrs; s < info->sechdrs + info->hdr.e_shnum; s++) { + /* Apply per-object .klp.arch sections */ + cnt = sscanf(info->secstrings + s->sh_name, + ".klp.arch.%55[^.].%127s", + sec_objname, secname); + if (cnt != 2) + continue; + if (strcmp(sec_objname, objname)) + continue; + if (!strcmp(".altinstructions", secname))The previous version of the patch compared against "altinstructions" (without the dot). I admit that I haven't tested it but the dot looks suspicious here.
Good eye, I should have explained why the dot is needed in the strcmp.. So, the new documentation states that any arch-specific sections to be applied by livepatch are to be prefixed with the string ".klp.arch.$objname.", note the required dot at the end of this prefix. So for example, if we have a .parainstructions section with a patch for the kvm module, the prefixed section name would look like: .klp.arch.kvm..parainstructions ^ prefix ^^ original name ^ That extra dot looks weird, but it is needed when we have section names like "__ftr_fixup" on powerpc. Without the extra dot at the end of ".klp.arch.$objname." We'd get names like ".klp.arch.$objname__ftr_fixup", and we wouldn't be able to tell where the objname ends and where the section name begins. But with ".klp.arch.$objname.__ftr_fixup", we have a hard delimeter and know that after the dot after $objname comes the original section name. Hope that helps! Jessica -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
