Messages in this thread | | | Date | Tue, 09 Dec 2014 15:47:56 +0900 | From | Masami Hiramatsu <> | Subject | Re: [RESEND][PATCH v15 7/7] ARM: kprobes: enable OPTPROBES for ARM 32 |
| |
(2014/12/08 23:09), Wang Nan wrote: > This patch introduce kprobeopt for ARM 32. > > Limitations: > - Currently only kernel compiled with ARM ISA is supported. > > - Offset between probe point and optinsn slot must not larger than > 32MiB. Masami Hiramatsu suggests replacing 2 words, it will make > things complex. Futher patch can make such optimization. > > Kprobe opt on ARM is relatively simpler than kprobe opt on x86 because > ARM instruction is always 4 bytes aligned and 4 bytes long. This patch > replace probed instruction by a 'b', branch to trampoline code and then > calls optimized_callback(). optimized_callback() calls opt_pre_handler() > to execute kprobe handler. It also emulate/simulate replaced instruction. > > When unregistering kprobe, the deferred manner of unoptimizer may leave > branch instruction before optimizer is called. Different from x86_64, > which only copy the probed insn after optprobe_template_end and > reexecute them, this patch call singlestep to emulate/simulate the insn > directly. Futher patch can optimize this behavior. > > Signed-off-by: Wang Nan <wangnan0@huawei.com> > Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> > Cc: Jon Medhurst (Tixy) <tixy@linaro.org> > Cc: Russell King - ARM Linux <linux@arm.linux.org.uk> > Cc: Will Deacon <will.deacon@arm.com> > --- > v1 -> v2: > - Improvement: if replaced instruction is conditional, generate a > conditional branch instruction for it; > - Introduces RELATIVEJUMP_OPCODES due to ARM kprobe_opcode_t is 4 > bytes; > - Removes size field in struct arch_optimized_insn; > - Use arm_gen_branch() to generate branch instruction; > - Remove all recover logic: ARM doesn't use tail buffer, no need > recover replaced instructions like x86; > - Remove incorrect CONFIG_THUMB checking; > - can_optimize() always returns true if address is well aligned; > - Improve optimized_callback: using opt_pre_handler(); > - Bugfix: correct range checking code and improve comments; > - Fix commit message. > > v2 -> v3: > - Rename RELATIVEJUMP_OPCODES to MAX_COPIED_INSNS; > - Remove unneeded checking: > arch_check_optimized_kprobe(), can_optimize(); > - Add missing flush_icache_range() in arch_prepare_optimized_kprobe(); > - Remove unneeded 'return;'. > > v3 -> v4: > - Use __mem_to_opcode_arm() to translate copied_insn to ensure it > works in big endian kernel; > - Replace 'nop' placeholder in trampoline code template with > '.long 0' to avoid confusion: reader may regard 'nop' as an > instruction, but it is value in fact. > > v4 -> v5: > - Don't optimize stack store operations. > - Introduce prepared field to arch_optimized_insn to indicate whether > it is prepared. Similar to size field with x86. See v1 -> v2. > > v5 -> v6: > - Dynamically reserve stack according to instruction. > - Rename: kprobes-opt.c -> kprobes-opt-arm.c. > - Set op->optinsn.insn after all works are done. > > v6 -> v7: > - Using checker to check stack consumption. > > v7 -> v8: > - Small code adjustments. > > v8 -> v9: > - Utilize original kprobe passed to arch_prepare_optimized_kprobe() > to avoid copy ainsn twice. > - A bug in arch_prepare_optimized_kprobe() is found and fixed. > > v9 -> v10: > - Commit message improvements. > > v10 -> v11: > - Move to arch/arm/probes/, insn.h is moved to arch/arm/include/asm. > - Code cleanup. > - Bugfix based on Tixy's test result: > - Trampoline deal with ARM -> Thumb transision instructions and > AEABI stack alignment requirement correctly. > - Trampoline code buffer should start at 4 byte aligned address. > We enforces it in this series by using macro to wrap 'code' var. > > v11 -> v12: > - Remove trampoline code stack trick and use r4 to save original > stack. > - Remove trampoline code buffer alignment trick. > - Names of files are changed. > > v12 -> v13: > - Assume stack always aligned by 4-bytes in any case. > - Comments update. > > v13 -> v14: > - Use stop_machine to wrap arch_optimize_kprobes to avoid a racing. > > v14 -> v15: > - In v14, stop_machine() wraps a coarse-grained piece of code which > makes things complex than it should be. In this version, > kprobes_remove_breakpoint() is extracted, core.c and opt-arm.c use it > to replace breakpoint to valid instruction. stop_machine() used inside > in kprobes_remove_breakpoint(). > --- > arch/arm/Kconfig | 1 + > arch/arm/{kernel => include/asm}/insn.h | 0 > arch/arm/include/asm/kprobes.h | 29 +++ > arch/arm/kernel/Makefile | 2 +- > arch/arm/kernel/ftrace.c | 3 +- > arch/arm/kernel/jump_label.c | 3 +- > arch/arm/probes/kprobes/Makefile | 1 + > arch/arm/probes/kprobes/core.c | 26 ++- > arch/arm/probes/kprobes/core.h | 2 + > arch/arm/probes/kprobes/opt-arm.c | 317 ++++++++++++++++++++++++++++++++ > 10 files changed, 372 insertions(+), 12 deletions(-) > rename arch/arm/{kernel => include/asm}/insn.h (100%) > create mode 100644 arch/arm/probes/kprobes/opt-arm.c > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 89c4b5c..2471240 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -59,6 +59,7 @@ config ARM > select HAVE_MEMBLOCK > select HAVE_MOD_ARCH_SPECIFIC if ARM_UNWIND > select HAVE_OPROFILE if (HAVE_PERF_EVENTS) > + select HAVE_OPTPROBES if !THUMB2_KERNEL > select HAVE_PERF_EVENTS > select HAVE_PERF_REGS > select HAVE_PERF_USER_STACK_DUMP > diff --git a/arch/arm/kernel/insn.h b/arch/arm/include/asm/insn.h > similarity index 100% > rename from arch/arm/kernel/insn.h > rename to arch/arm/include/asm/insn.h > diff --git a/arch/arm/include/asm/kprobes.h b/arch/arm/include/asm/kprobes.h > index 56f9ac6..50ff3bc 100644 > --- a/arch/arm/include/asm/kprobes.h > +++ b/arch/arm/include/asm/kprobes.h > @@ -50,5 +50,34 @@ int kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr); > int kprobe_exceptions_notify(struct notifier_block *self, > unsigned long val, void *data); > > +/* optinsn template addresses */ > +extern __visible kprobe_opcode_t optprobe_template_entry; > +extern __visible kprobe_opcode_t optprobe_template_val; > +extern __visible kprobe_opcode_t optprobe_template_call; > +extern __visible kprobe_opcode_t optprobe_template_end; > +extern __visible kprobe_opcode_t optprobe_template_sub_sp; > +extern __visible kprobe_opcode_t optprobe_template_add_sp; > + > +#define MAX_OPTIMIZED_LENGTH 4 > +#define MAX_OPTINSN_SIZE \ > + ((unsigned long)&optprobe_template_end - \ > + (unsigned long)&optprobe_template_entry) > +#define RELATIVEJUMP_SIZE 4 > + > +struct arch_optimized_insn { > + /* > + * copy of the original instructions. > + * Different from x86, ARM kprobe_opcode_t is u32. > + */ > +#define MAX_COPIED_INSN DIV_ROUND_UP(RELATIVEJUMP_SIZE, sizeof(kprobe_opcode_t)) > + kprobe_opcode_t copied_insn[MAX_COPIED_INSN]; > + /* detour code buffer */ > + kprobe_opcode_t *insn; > + /* > + * We always copy one instruction on ARM, > + * so size will always be 4, and unlike x86, there is no > + * need for a size field. > + */ > +}; > > #endif /* _ARM_KPROBES_H */ > diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile > index 40d3e00..1d0f4e7 100644 > --- a/arch/arm/kernel/Makefile > +++ b/arch/arm/kernel/Makefile > @@ -52,7 +52,7 @@ obj-$(CONFIG_FUNCTION_GRAPH_TRACER) += ftrace.o insn.o > obj-$(CONFIG_JUMP_LABEL) += jump_label.o insn.o patch.o > obj-$(CONFIG_KEXEC) += machine_kexec.o relocate_kernel.o > # Main staffs in KPROBES are in arch/arm/probes/ . > -obj-$(CONFIG_KPROBES) += patch.o > +obj-$(CONFIG_KPROBES) += patch.o insn.o > obj-$(CONFIG_OABI_COMPAT) += sys_oabi-compat.o > obj-$(CONFIG_ARM_THUMBEE) += thumbee.o > obj-$(CONFIG_KGDB) += kgdb.o > diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c > index af9a8a9..ec7e332 100644 > --- a/arch/arm/kernel/ftrace.c > +++ b/arch/arm/kernel/ftrace.c > @@ -19,8 +19,7 @@ > #include <asm/cacheflush.h> > #include <asm/opcodes.h> > #include <asm/ftrace.h> > - > -#include "insn.h" > +#include <asm/insn.h> > > #ifdef CONFIG_THUMB2_KERNEL > #define NOP 0xf85deb04 /* pop.w {lr} */ > diff --git a/arch/arm/kernel/jump_label.c b/arch/arm/kernel/jump_label.c > index c6c73ed..35a8fbb 100644 > --- a/arch/arm/kernel/jump_label.c > +++ b/arch/arm/kernel/jump_label.c > @@ -1,8 +1,7 @@ > #include <linux/kernel.h> > #include <linux/jump_label.h> > #include <asm/patch.h> > - > -#include "insn.h" > +#include <asm/insn.h> > > #ifdef HAVE_JUMP_LABEL > > diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile > index bc8d504..76a36bf 100644 > --- a/arch/arm/probes/kprobes/Makefile > +++ b/arch/arm/probes/kprobes/Makefile > @@ -7,5 +7,6 @@ obj-$(CONFIG_KPROBES) += actions-thumb.o checkers-thumb.o > test-kprobes-objs += test-thumb.o > else > obj-$(CONFIG_KPROBES) += actions-arm.o checkers-arm.o > +obj-$(CONFIG_OPTPROBES) += opt-arm.o > test-kprobes-objs += test-arm.o > endif > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c > index 3a58db4..a4ec240 100644 > --- a/arch/arm/probes/kprobes/core.c > +++ b/arch/arm/probes/kprobes/core.c > @@ -163,19 +163,31 @@ void __kprobes arch_arm_kprobe(struct kprobe *p) > * memory. It is also needed to atomically set the two half-words of a 32-bit > * Thumb breakpoint. > */ > -int __kprobes __arch_disarm_kprobe(void *p) > -{ > - struct kprobe *kp = p; > - void *addr = (void *)((uintptr_t)kp->addr & ~1); > - > - __patch_text(addr, kp->opcode); > +struct patch { > + void *addr; > + unsigned int insn; > +}; > > +static int __kprobes_remove_breakpoint(void *data) > +{ > + struct patch *p = data; > + __patch_text(p->addr, p->insn); > return 0; > } > > +void __kprobes kprobes_remove_breakpoint(void *addr, unsigned int insn) > +{ > + struct patch p = { > + .addr = addr, > + .insn = insn, > + }; > + stop_machine(__kprobes_remove_breakpoint, &p, cpu_online_mask); > +}
Hmm, I think finally we should fix patch_text() in patch.c to forcibly use stop_machine by adding "bool stop" parameter, instead of introducing new another patch_text() implementation, because we'd better avoid two private "patch" data structures. Maybe someday we can find another better solution for self modifying. (I actually doesn't like stop_machine() which disturbs real-time processing)
The other parts look good to me :)
Thank you!
-- Masami HIRAMATSU Software Platform Research Dept. Linux Technology Research Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: masami.hiramatsu.pt@hitachi.com
| |