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.
> + alt = s;
> + if (!strcmp(".parainstructions", secname))
Same here.
Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
