This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
ftrace_lookup_mcount() will return an address of mcount call in a function.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- include/linux/ftrace.h | 2 ++ kernel/trace/ftrace.c | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h index 1da6029..1d0271f 100644 --- a/include/linux/ftrace.h +++ b/include/linux/ftrace.h @@ -699,6 +699,8 @@ static inline void __ftrace_enabled_restore(int enabled) # define trace_preempt_off(a0, a1) do { } while (0) #endif
+extern unsigned long ftrace_lookup_mcount(unsigned long addr); + #ifdef CONFIG_FTRACE_MCOUNT_RECORD extern void ftrace_init(void); #else diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index 4f22802..fcfb04f 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c @@ -4857,6 +4857,32 @@ static int ftrace_process_locs(struct module *mod, return ret; }
+unsigned long ftrace_lookup_mcount(unsigned long addr) +{ + struct ftrace_page *pg; + struct dyn_ftrace *rec; + char str[KSYM_SYMBOL_LEN]; + char *modname; + const char *name; + unsigned long mcount = 0; + unsigned long offset; + + mutex_lock(&ftrace_lock); + + do_for_each_ftrace_rec(pg, rec) { + name = kallsyms_lookup(rec->ip, NULL, &offset, &modname, str); + if (name && rec->ip == (addr + offset)) { + mcount = rec->ip; + goto out_unlock; + } + } while_for_each_ftrace_rec(); + + out_unlock: + mutex_unlock(&ftrace_lock); + + return mcount; +} + #ifdef CONFIG_MODULES
#define next_to_ftrace_page(p) container_of(p, struct ftrace_page, next)
The existing livepatch code assumes that a fentry call is inserted before a function prolog by -mfentry option of gcc. But the location of mcount() can be identified by using ftrace_lookup_mcount(), and which eventually allows arch's which don't support -mfentry option, like arm64, to support livepatch.
This patch modifies core and x86 code before adding livepatch support for arm64.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/x86/include/asm/livepatch.h | 5 +++++ include/linux/livepatch.h | 2 ++ kernel/livepatch/core.c | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/livepatch.h b/arch/x86/include/asm/livepatch.h index a455a53..914954a 100644 --- a/arch/x86/include/asm/livepatch.h +++ b/arch/x86/include/asm/livepatch.h @@ -39,6 +39,11 @@ static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip) { regs->ip = ip; } + +static inline unsigned long klp_arch_lookup_mcount(unsigned long addr) +{ + return addr; +} #else #error Live patching support is disabled; check CONFIG_LIVEPATCH #endif diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h index 95023fd..fec7800 100644 --- a/include/linux/livepatch.h +++ b/include/linux/livepatch.h @@ -47,6 +47,7 @@ struct klp_func { /* external */ const char *old_name; void *new_func; + unsigned long new_func_mcount; /* * The old_addr field is optional and can be used to resolve * duplicate symbol names in the vmlinux object. If this @@ -56,6 +57,7 @@ struct klp_func { * way to resolve the ambiguity. */ unsigned long old_addr; + unsigned long old_addr_mcount;
/* internal */ struct kobject kobj; diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c index 3f9f1d6..3c7c8070 100644 --- a/kernel/livepatch/core.c +++ b/kernel/livepatch/core.c @@ -245,6 +245,9 @@ static int klp_find_verify_func_addr(struct klp_object *obj, ret = klp_verify_vmlinux_symbol(func->old_name, func->old_addr);
+ if (func->old_addr && !ret) + func->old_addr_mcount = klp_arch_lookup_mcount(func->old_addr); + return ret; }
@@ -330,7 +333,7 @@ static void notrace klp_ftrace_handler(unsigned long ip, if (WARN_ON_ONCE(!func)) goto unlock;
- klp_arch_set_pc(regs, (unsigned long)func->new_func); + klp_arch_set_pc(regs, func->new_func_mcount); unlock: rcu_read_unlock(); } @@ -358,7 +361,8 @@ static int klp_disable_func(struct klp_func *func) return ret; }
- ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0); + ret = ftrace_set_filter_ip(&ops->fops, + func->old_addr_mcount, 1, 0); if (ret) pr_warn("function unregister succeeded but failed to clear the filter\n");
@@ -401,7 +405,8 @@ static int klp_enable_func(struct klp_func *func) INIT_LIST_HEAD(&ops->func_stack); list_add_rcu(&func->stack_node, &ops->func_stack);
- ret = ftrace_set_filter_ip(&ops->fops, func->old_addr, 0, 0); + ret = ftrace_set_filter_ip(&ops->fops, + func->old_addr_mcount, 0, 0); if (ret) { pr_err("failed to set ftrace filter for function '%s' (%d)\n", func->old_name, ret); @@ -412,7 +417,8 @@ static int klp_enable_func(struct klp_func *func) if (ret) { pr_err("failed to register ftrace handler for function '%s' (%d)\n", func->old_name, ret); - ftrace_set_filter_ip(&ops->fops, func->old_addr, 1, 0); + ftrace_set_filter_ip(&ops->fops, + func->old_addr_mcount, 1, 0); goto err; }
@@ -742,6 +748,8 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) { INIT_LIST_HEAD(&func->stack_node); func->state = KLP_DISABLED; + func->new_func_mcount = + klp_arch_lookup_mcount((unsigned long)func->new_func);
return kobject_init_and_add(&func->kobj, &klp_ktype_func, obj->kobj, "%s", func->old_name);
CONFIG_DYNAMIC_TRACE_WITH_REGS is a prerequisite for ftrace-based kprobes as well as livepatch. This patch adds ftrace_regs_caller(), which will pass pt_regs info to ftrace handlers, to enable this config.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 +++++++- 4 files changed, 152 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 1b8e973..c3678ed 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -54,6 +54,7 @@ config ARM64 select HAVE_DMA_ATTRS select HAVE_DMA_CONTIGUOUS select HAVE_DYNAMIC_FTRACE + select HAVE_DYNAMIC_FTRACE_WITH_REGS select HAVE_EFFICIENT_UNALIGNED_ACCESS select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_TRACER diff --git a/arch/arm64/include/asm/ftrace.h b/arch/arm64/include/asm/ftrace.h index c5534fa..5a14269 100644 --- a/arch/arm64/include/asm/ftrace.h +++ b/arch/arm64/include/asm/ftrace.h @@ -16,6 +16,10 @@ #define MCOUNT_ADDR ((unsigned long)_mcount) #define MCOUNT_INSN_SIZE AARCH64_INSN_SIZE
+#ifdef CONFIG_DYNAMIC_FTRACE +#define ARCH_SUPPORTS_FTRACE_OPS 1 +#endif + #ifndef __ASSEMBLY__ #include <linux/compat.h>
diff --git a/arch/arm64/kernel/entry-ftrace.S b/arch/arm64/kernel/entry-ftrace.S index 08cafc5..fed28eb 100644 --- a/arch/arm64/kernel/entry-ftrace.S +++ b/arch/arm64/kernel/entry-ftrace.S @@ -10,6 +10,7 @@ */
#include <linux/linkage.h> +#include <asm/asm-offsets.h> #include <asm/ftrace.h> #include <asm/insn.h>
@@ -86,6 +87,95 @@ add \reg, \reg, #8 .endm
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +/* + * stack layout after mcount_save_regs in _mcount(): + * + * current sp/fp => 0:+---------+ + * in _mcount() |pt_regs | + * | | + * | +-----+ + * | | x29 | -> instrumented function's fp + * | +-----+ + * | | x30 | -> _mcount()'s lr + * | +-----+ (= instrumented function's pc) + * +S_FRAME_SIZE | | + * old sp => :+-----+--- + * when instrumented | | + * function calls | ... | + * _mcount() | | + * | | + * instrumented => +xx:+-----+ + * function's fp | x29 | -> parent's fp + * +-----+ + * | x30 | -> instrumented function's lr (= parent's pc) + * +-----+ + * | ... | + */ + .macro mcount_save_regs + sub sp, sp, #S_FRAME_SIZE + + stp x0, x1, [sp, #8 * 0] + stp x2, x3, [sp, #8 * 2] + stp x4, x5, [sp, #8 * 4] + stp x6, x7, [sp, #8 * 6] + stp x8, x9, [sp, #8 * 8] + stp x10, x11, [sp, #8 * 10] + stp x12, x13, [sp, #8 * 12] + stp x14, x15, [sp, #8 * 14] + stp x16, x17, [sp, #8 * 16] + stp x18, x19, [sp, #8 * 18] + stp x20, x21, [sp, #8 * 20] + stp x22, x23, [sp, #8 * 22] + stp x24, x25, [sp, #8 * 24] + stp x26, x27, [sp, #8 * 26] + stp x28, x29, [sp, #8 * 28] + + ldr x0, [x29, #8] + mcount_adjust_addr x0, x0 + str x0, [sp, #S_LR] + + add x0, sp, #S_FRAME_SIZE + str x0, [sp, #S_SP] + + mcount_adjust_addr x0, x30 + str x0, [sp, #S_PC] + + ldr x0, [sp, #8 * 0] + + mov x29, sp + .endm + + /* for instrumented function */ + .macro mcount_get_lr1 reg + ldr \reg, [x29, #8 * 29] + ldr \reg, [\reg, #8] + mcount_adjust_addr \reg, \reg + .endm + + .macro mcount_restore_rest_regs + ldp x0, x1, [sp, #8 * 0] + ldp x2, x3, [sp, #8 * 2] + ldp x4, x5, [sp, #8 * 4] + ldp x6, x7, [sp, #8 * 6] + ldp x8, x9, [sp, #8 * 8] + ldp x10, x11, [sp, #8 * 10] + ldp x12, x13, [sp, #8 * 12] + ldp x14, x15, [sp, #8 * 14] + ldp x16, x17, [sp, #8 * 16] + ldp x18, x19, [sp, #8 * 18] + ldp x20, x21, [sp, #8 * 20] + ldp x22, x23, [sp, #8 * 22] + ldp x24, x25, [sp, #8 * 24] + ldp x26, x27, [sp, #8 * 26] + ldp x28, x29, [sp, #8 * 28] + ldr x30, [sp, #S_LR] + add sp, sp, #S_FRAME_SIZE + + mcount_enter + .endm +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ + #ifndef CONFIG_DYNAMIC_FTRACE /* * void _mcount(unsigned long return_address) @@ -156,6 +246,10 @@ ENTRY(ftrace_caller)
mcount_get_pc0 x0 // function's pc mcount_get_lr x1 // function's lr + adrp x2, function_trace_op + add x2, x2, #:lo12:function_trace_op + ldr x2, [x2] // current ftrace_ops + mov x3, xzr // pt_regs (NULL)
.global ftrace_call ftrace_call: // tracer(pc, lr); @@ -171,6 +265,36 @@ ftrace_graph_call: // ftrace_graph_caller();
mcount_exit ENDPROC(ftrace_caller) + +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +/* + * void ftrace_regs_caller() + */ +ENTRY(ftrace_regs_caller) + mcount_save_regs + + mcount_get_pc0 x0 // function's pc + mcount_get_lr1 x1 // function's lr + adrp x2, function_trace_op + add x2, x2, #:lo12:function_trace_op + ldr x2, [x2] // current ftrace_ops + mov x3, sp // pt_regs + + .global ftrace_regs_call +ftrace_regs_call: // tracer(pc, lr, ops, regs); + nop // This will be replaced with "bl xxx" + // where xxx can be any kind of tracer. + + mcount_restore_rest_regs + +#ifdef CONFIG_FUNCTION_GRAPH_TRACER + b ftrace_graph_call +#endif + + mcount_exit +ENDPROC(ftrace_regs_caller) +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ + #endif /* CONFIG_DYNAMIC_FTRACE */
ENTRY(ftrace_stub) diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c index c851be7..5440673 100644 --- a/arch/arm64/kernel/ftrace.c +++ b/arch/arm64/kernel/ftrace.c @@ -56,12 +56,21 @@ int ftrace_update_ftrace_func(ftrace_func_t func) { unsigned long pc; u32 new; + int ret;
pc = (unsigned long)&ftrace_call; new = aarch64_insn_gen_branch_imm(pc, (unsigned long)func, AARCH64_INSN_BRANCH_LINK); + ret = ftrace_modify_code(pc, 0, new, false);
- return ftrace_modify_code(pc, 0, new, false); + if (!ret) { + pc = (unsigned long)&ftrace_regs_call; + new = aarch64_insn_gen_branch_imm(pc, (unsigned long)func, + AARCH64_INSN_BRANCH_LINK); + ret = ftrace_modify_code(pc, 0, new, false); + } + + return ret; }
/* @@ -97,6 +106,19 @@ int __init ftrace_dyn_arch_init(void) { return 0; } + +int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, + unsigned long addr) +{ + unsigned long pc = rec->ip; + u32 old, new; + + old = aarch64_insn_gen_branch_imm(pc, old_addr, + AARCH64_INSN_BRANCH_LINK); + new = aarch64_insn_gen_branch_imm(pc, addr, AARCH64_INSN_BRANCH_LINK); + + return ftrace_modify_code(pc, old, new, true); +} #endif /* CONFIG_DYNAMIC_FTRACE */
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org --- arch/arm64/Kconfig | 3 ++ arch/arm64/include/asm/livepatch.h | 38 ++++++++++++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++++++++++++++++++ 4 files changed, 110 insertions(+) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index c3678ed..d4b5bac 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -61,6 +61,7 @@ config ARM64 select HAVE_FUNCTION_GRAPH_TRACER select HAVE_GENERIC_DMA_COHERENT select HAVE_HW_BREAKPOINT if PERF_EVENTS + select HAVE_LIVEPATCH select HAVE_MEMBLOCK select HAVE_PATA_PLATFORM select HAVE_PERF_EVENTS @@ -612,6 +613,8 @@ config SETEND_EMULATION If unsure, say Y endif
+source "kernel/livepatch/Kconfig" + endmenu
menu "Boot options" diff --git a/arch/arm64/include/asm/livepatch.h b/arch/arm64/include/asm/livepatch.h new file mode 100644 index 0000000..590d139 --- /dev/null +++ b/arch/arm64/include/asm/livepatch.h @@ -0,0 +1,38 @@ +/* + * Copyright (C) 2015 Linaro Limited + * Author: AKASHI Takahiro takahiro.akashi@linaro.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#ifndef __ASM_LIVEPATCH_H +#define __ASM_LIVEPATCH_H + +#include <linux/module.h> +#include <linux/ftrace.h> + +#ifdef CONFIG_LIVEPATCH +static inline int klp_check_compiler_support(void) +{ + return 0; +} +extern int klp_write_module_reloc(struct module *mod, unsigned long type, + unsigned long loc, unsigned long value); + +extern unsigned long ftrace_lookup_mcount(unsigned long addr); + +static inline void klp_arch_set_pc(struct pt_regs *regs, unsigned long ip) +{ + regs->regs[30] = ip; +} + +static inline unsigned long klp_arch_lookup_mcount(unsigned long addr) +{ + return ftrace_lookup_mcount(addr); +} +#else +#error Live patching support is disabled; check CONFIG_LIVEPATCH +#endif + +#endif /* __ASM_LIVEPATCH_H */ diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile index 5ee07ee..7614990 100644 --- a/arch/arm64/kernel/Makefile +++ b/arch/arm64/kernel/Makefile @@ -35,6 +35,7 @@ arm64-obj-$(CONFIG_KGDB) += kgdb.o arm64-obj-$(CONFIG_EFI) += efi.o efi-stub.o efi-entry.o arm64-obj-$(CONFIG_PCI) += pci.o arm64-obj-$(CONFIG_ARMV8_DEPRECATED) += armv8_deprecated.o +arm64-obj-$(CONFIG_LIVEPATCH) += livepatch.o
obj-y += $(arm64-obj-y) vdso/ obj-m += $(arm64-obj-m) diff --git a/arch/arm64/kernel/livepatch.c b/arch/arm64/kernel/livepatch.c new file mode 100644 index 0000000..abe4947 --- /dev/null +++ b/arch/arm64/kernel/livepatch.c @@ -0,0 +1,68 @@ +/* + * livepatch.c - arm64-specific Kernel Live Patching Core + * + * Copyright (C) 2015 Linaro Limited + * Author: AKASHI Takahiro takahiro.akashi@linaro.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/module.h> +#include <asm/cacheflush.h> +#include <asm/elf.h> +#include <asm/insn.h> +#include <asm/livepatch.h> + +/** + * klp_write_module_reloc() - write a relocation in a module + * @mod: module in which the section to be modified is found + * @type: ELF relocation type (see asm/elf.h) + * @loc: address that the relocation should be written to + * @value: relocation value (sym address + addend) + * + * This function writes a relocation to the specified location for + * a particular module. + */ +int klp_write_module_reloc(struct module *mod, unsigned long type, + unsigned long loc, unsigned long value) +{ + unsigned long core = (unsigned long)mod->module_core; + unsigned long core_ro_size = mod->core_ro_size; + unsigned long core_size = mod->core_size; + bool readonly; + u32 new; + int ret; + + switch (type) { + case R_AARCH64_NONE: + return 0; + case R_AARCH64_CALL26: + break; + default: + /* unsupported relocation type */ + return -EINVAL; + } + + if (loc < core || loc >= core + core_size) + /* loc does not point to any symbol inside the module */ + return -EINVAL; + + if (loc < core + core_ro_size) + readonly = true; + else + readonly = false; + + if (readonly) + set_memory_rw(loc & PAGE_MASK, 1); + + new = aarch64_insn_gen_branch_imm(loc, value, + AARCH64_INSN_BRANCH_NOLINK); + ret = aarch64_insn_patch_text_nosync((void *)loc, new); + + if (readonly) + set_memory_ro(loc & PAGE_MASK, 1); + + return ret; +}
On Fri, 24 Apr 2015 11:44:05 +0900 AKASHI Takahiro takahiro.akashi@linaro.org wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
You need to be extremely careful about this. I don't know what arm does with mcount but on x86 without -fentry, it sets up the stack frame before calling mcount. That is, if mcount returns to a different function, it doesn't mean that the registers will match the parameters.
I have to look at what arm64 does when compiled with -pg. Because, if it moves registers around before the mcount, you will have a disaster on your hands if it returns to a different function that moved the registers around differently.
Also, you need to be careful about how link registers are handled.
-- Steve
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
On 2015/4/24 10:44, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
This is not correct for the case that the prologue of the old and new function is different. Thanks, Li Bin
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
(2015/04/24 12:24), Li Bin wrote:
On 2015/4/24 10:44, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
This is not correct for the case that the prologue of the old and new function is different.
Hmm, is that possible to support -mfentry on arm64?
Of course we can not call a function directly at the first instruction of functions on arm, because it can overwrite link register which stores caller address. However, we can do "store link register to stack and branch with link" on arm. That is actually almost same as -mfentry does :), and that may not depend on the prologue.
Thank you,
Thanks, Li Bin
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
Steve, Li and Masami,
Thank you for all your comments. You pointed out the cases that I didn't think of. Let me think how I can manage the issues for a while. Probably I will talk to gcc guys.
-Takahiro AKASHI
On 04/24/2015 03:05 PM, Masami Hiramatsu wrote:
(2015/04/24 12:24), Li Bin wrote:
On 2015/4/24 10:44, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
This is not correct for the case that the prologue of the old and new function is different.
Hmm, is that possible to support -mfentry on arm64?
Of course we can not call a function directly at the first instruction of functions on arm, because it can overwrite link register which stores caller address. However, we can do "store link register to stack and branch with link" on arm. That is actually almost same as -mfentry does :), and that may not depend on the prologue.
Thank you,
Thanks, Li Bin
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
Thanks,
Sorry for not replying soon.
On 04/24/2015 06:27 PM, Jiri Kosina wrote:
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
As other people said, my current patch has some drawbacks and was far from perfect. I talked to a toolchain guy in Linaro, and he suggested that, instead of x86 specific -mfentry option, we should add a new generic option, -fprolog-pad=N.
It works as if "-pg -mfentry -mrecord-mcount -mnop-mcount" were specified on a command line, but actually puts N nop instructions at the very beginning of a function. (Please note that -mrecord-mcount and -mnop-mcount options have been added in x86 gcc to make it easier to implement ftrace and others.)
While it is not clear that this new option will be implemented soon (in gcc-6), once it is supported on aarch64, I will update and re-post my livepatch patch as well as DYNAMIC_TRACE_WITH_REGS ftrace.
Thanks, -Takahiro AKASHI
Thanks,
On Wed, 27 May 2015, AKASHI Takahiro wrote:
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
As other people said, my current patch has some drawbacks and was far from perfect. I talked to a toolchain guy in Linaro, and he suggested that, instead of x86 specific -mfentry option, we should add a new generic option, -fprolog-pad=N.
It works as if "-pg -mfentry -mrecord-mcount -mnop-mcount" were specified on a command line, but actually puts N nop instructions at the very beginning of a function. (Please note that -mrecord-mcount and -mnop-mcount options have been added in x86 gcc to make it easier to implement ftrace and others.)
This is BTW implemented in gcc for various architectures already (see -mhotpatch for s390 or -mprofile-kernel for ppc64). Would be nice to at least try the semantics and format unified in some sense.
Thanks,
On 2015/05/27 15:09, AKASHI Takahiro wrote:
Sorry for not replying soon.
On 04/24/2015 06:27 PM, Jiri Kosina wrote:
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
As other people said, my current patch has some drawbacks and was far from perfect. I talked to a toolchain guy in Linaro, and he suggested that, instead of x86 specific -mfentry option, we should add a new generic option, -fprolog-pad=N.
It works as if "-pg -mfentry -mrecord-mcount -mnop-mcount" were specified on a command line, but actually puts N nop instructions at the very beginning of a function. (Please note that -mrecord-mcount and -mnop-mcount options have been added in x86 gcc to make it easier to implement ftrace and others.)
Ah, that's a cool idea :) Note that it also will need to take care of debuginfo. Previously -mfentry had broken the debuginfo about function prologue.
While it is not clear that this new option will be implemented soon (in gcc-6), once it is supported on aarch64, I will update and re-post my livepatch patch as well as DYNAMIC_TRACE_WITH_REGS ftrace.
Thank you!
On 2015/4/24 14:05, Masami Hiramatsu wrote:
(2015/04/24 12:24), Li Bin wrote:
On 2015/4/24 10:44, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
This is not correct for the case that the prologue of the old and new function is different.
Hmm, is that possible to support -mfentry on arm64?
Of course we can not call a function directly at the first instruction of functions on arm, because it can overwrite link register which stores caller address. However, we can do "store link register to stack and branch with link" on arm. That is actually almost same as -mfentry does :), and that may not depend on the prologue.
Yes, but the method "store link register to stack" will break the arm64 ABI rules, and memory operations may bring performance cost. We have a gcc -mfentry implementation strategy for arm64, and based on this we implement the livepatch for arm64. I will post the patchset for review soon. Thank you, Li Bin
Thank you,
Thanks, Li Bin
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
Please note that I tested the feature only with livepatch-sample, and the code for DYNAMIC_TRACE_WITH_REGS is still rough-edged.
To: Steven Rostedt rostedt@goodmis.org To: Ingo Molnar mingo@kernel.org To: Josh Poimboeuf jpoimboe@redhat.com To: Seth Jennings sjenning@redhat.com To: Jiri Kosina jkosina@suse.cz To: Vojtech Pavlik vojtech@suse.cz To: Catalin Marinas catalin.marinas@arm.com To: Will Deacon will.deacon@arm.com
AKASHI Takahiro (4): ftrace: add a helper function for livepatch livepatch: adjust a patched function's address arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version arm64: add livepatch support
arch/arm64/Kconfig | 4 ++ arch/arm64/include/asm/ftrace.h | 4 ++ arch/arm64/include/asm/livepatch.h | 38 +++++++++++ arch/arm64/kernel/Makefile | 1 + arch/arm64/kernel/entry-ftrace.S | 124 ++++++++++++++++++++++++++++++++++++ arch/arm64/kernel/ftrace.c | 24 ++++++- arch/arm64/kernel/livepatch.c | 68 ++++++++++++++++++++ arch/x86/include/asm/livepatch.h | 5 ++ include/linux/ftrace.h | 2 + include/linux/livepatch.h | 2 + kernel/livepatch/core.c | 16 +++-- kernel/trace/ftrace.c | 26 ++++++++ 12 files changed, 309 insertions(+), 5 deletions(-) create mode 100644 arch/arm64/include/asm/livepatch.h create mode 100644 arch/arm64/kernel/livepatch.c
On 2015/4/24 17:27, Jiri Kosina wrote:
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
The function prologue on arm64 with gcc -pg look like as following: func: stp x29, x30, [sp, -48]! add x29, sp, 0 mov x1, x30 str w0, [x29,28] mov x0, x1 bl _mcount ... Thanks, Li Bin
Thanks,
On Thu, May 28, 2015 at 07:08:42AM +0100, Li Bin wrote:
On 2015/4/24 17:27, Jiri Kosina wrote:
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
The function prologue on arm64 with gcc -pg look like as following: func: stp x29, x30, [sp, -48]! add x29, sp, 0 mov x1, x30 str w0, [x29,28] mov x0, x1 bl _mcount
Just for the avoidance of confusion, this looks like a function with a live parameter in x0, which explains the str to the stack and the juggling of x30 into x0. I don't think there's necessarily a golden template for the prologue code.
Will
On 05/28/2015 05:58 PM, Will Deacon wrote:
On Thu, May 28, 2015 at 07:08:42AM +0100, Li Bin wrote:
On 2015/4/24 17:27, Jiri Kosina wrote:
On Fri, 24 Apr 2015, AKASHI Takahiro wrote:
This patchset enables livepatch support on arm64.
Livepatch was merged in v4.0, and allows replacying a function dynamically based on ftrace framework, but it also requires -mfentry option of gcc. Currently arm64 gcc doesn't support it, but by adding a helper function to ftrace, we will be able to support livepatch on arch's which don't support this option.
I submit this patchset as RFC since I'm not quite sure that I'm doing in the right way, or we should definitely support -fentry instead.
I don't have arm64 cross-compiler handy, could you please copy/paste how does function prologue, generated by gcc -pg on arm64 look like?
The function prologue on arm64 with gcc -pg look like as following: func: stp x29, x30, [sp, -48]! add x29, sp, 0 mov x1, x30 str w0, [x29,28] mov x0, x1 bl _mcount
Just for the avoidance of confusion, this looks like a function with a live parameter in x0, which explains the str to the stack and the juggling of x30 into x0. I don't think there's necessarily a golden template for the prologue code.
We just started the discussion about gcc, -mfentry vs. a more generic option that I mentioned in: http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/346632.html on gcc's devel ML.
-Takahiro AKASHI
Will
linaro-kernel@lists.linaro.org
-
AKASHI Takahiro -
Jiri Kosina -
Li Bin -
Masami Hiramatsu -
Steven Rostedt -
Will Deacon