2015-04-24 02:44:46

by AKASHI Takahiro

[permalink] [raw]
Subject: [RFC 0/4] arm64: add livepatch support

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 <[email protected]>
To: Ingo Molnar <[email protected]>
To: Josh Poimboeuf <[email protected]>
To: Seth Jennings <[email protected]>
To: Jiri Kosina <[email protected]>
To: Vojtech Pavlik <[email protected]>
To: Catalin Marinas <[email protected]>
To: Will Deacon <[email protected]>

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

--
1.7.9.5


2015-04-24 02:45:01

by AKASHI Takahiro

[permalink] [raw]
Subject: [RFC 1/4] ftrace: add a helper function for livepatch

ftrace_lookup_mcount() will return an address of mcount call in a function.

Signed-off-by: AKASHI Takahiro <[email protected]>
---
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)
--
1.7.9.5

2015-04-24 02:45:10

by AKASHI Takahiro

[permalink] [raw]
Subject: [RFC 2/4] livepatch: adjust a patched function's address

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 <[email protected]>
---
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);
--
1.7.9.5

2015-04-24 02:45:20

by AKASHI Takahiro

[permalink] [raw]
Subject: [RFC 3/4] arm64: ftrace: add DYNAMIC_TRACE_WITH_REGS version

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 <[email protected]>
---
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
--
1.7.9.5

2015-04-24 02:45:30

by AKASHI Takahiro

[permalink] [raw]
Subject: [RFC 4/4] arm64: add livepatch support


Signed-off-by: AKASHI Takahiro <[email protected]>
---
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 <[email protected]>
+ *
+ * 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 <[email protected]>
+ *
+ * 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;
+}
--
1.7.9.5

2015-04-24 02:58:22

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC 0/4] arm64: add livepatch support

On Fri, 24 Apr 2015 11:44:05 +0900
AKASHI Takahiro <[email protected]> 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 <[email protected]>
> To: Ingo Molnar <[email protected]>
> To: Josh Poimboeuf <[email protected]>
> To: Seth Jennings <[email protected]>
> To: Jiri Kosina <[email protected]>
> To: Vojtech Pavlik <[email protected]>
> To: Catalin Marinas <[email protected]>
> To: Will Deacon <[email protected]>
>
> 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 03:25:15

by Li Bin

[permalink] [raw]
Subject: Re: [RFC 0/4] arm64: add livepatch support

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 <[email protected]>
> To: Ingo Molnar <[email protected]>
> To: Josh Poimboeuf <[email protected]>
> To: Seth Jennings <[email protected]>
> To: Jiri Kosina <[email protected]>
> To: Vojtech Pavlik <[email protected]>
> To: Catalin Marinas <[email protected]>
> To: Will Deacon <[email protected]>
>
> 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
>

Subject: Re: [RFC 0/4] arm64: add livepatch support

(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 <[email protected]>
>> To: Ingo Molnar <[email protected]>
>> To: Josh Poimboeuf <[email protected]>
>> To: Seth Jennings <[email protected]>
>> To: Jiri Kosina <[email protected]>
>> To: Vojtech Pavlik <[email protected]>
>> To: Catalin Marinas <[email protected]>
>> To: Will Deacon <[email protected]>
>>
>> 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
>>
>
>
>


--
Masami HIRAMATSU
Linux Technology Research Center, System Productivity Research Dept.
Center for Technology Innovation - Systems Engineering
Hitachi, Ltd., Research & Development Group
E-mail: [email protected]

2015-04-24 08:04:16

by AKASHI Takahiro

[permalink] [raw]
Subject: Re: [RFC 0/4] arm64: add livepatch support

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 <[email protected]>
>>> To: Ingo Molnar <[email protected]>
>>> To: Josh Poimboeuf <[email protected]>
>>> To: Seth Jennings <[email protected]>
>>> To: Jiri Kosina <[email protected]>
>>> To: Vojtech Pavlik <[email protected]>
>>> To: Catalin Marinas <[email protected]>
>>> To: Will Deacon <[email protected]>
>>>
>>> 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 09:27:35

by Jiri Kosina

[permalink] [raw]
Subject: Re: [RFC 0/4] arm64: add livepatch support

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,

--
Jiri Kosina
SUSE Labs