On 06/13/2016 02:50 AM, Masami Hiramatsu wrote:
On Mon, 13 Jun 2016 00:10:29 -0400
David Long <dave.l...@linaro.org> wrote:
---
arch/arm64/Kconfig | 1 +
arch/arm64/include/asm/debug-monitors.h | 5 +
arch/arm64/include/asm/insn.h | 4 +-
arch/arm64/include/asm/kprobes.h | 60 ++++
arch/arm64/include/asm/probes.h | 44 +++
arch/arm64/kernel/Makefile | 1 +
arch/arm64/kernel/debug-monitors.c | 18 +-
arch/arm64/kernel/kprobes-arm64.c | 144 +++++++++
arch/arm64/kernel/kprobes-arm64.h | 35 +++
arch/arm64/kernel/kprobes.c | 526
++++++++++++++++++++++++++++++++
Not sure why kprobes.c and kprobes-arm64.c are splitted.
This comes from the model of the arm32 kprobes code where handling of
the low-level instruction simulation is implemented in separate files
for 32-bit vs. thumb instructions. It should make a little more sense
in the future when additional instruction simulation code will hopefully
be added for those instructions we cannot currently single-step
out-of-line. It also probably *could* be merged into one file.
Hmm, at least the name of arch/arm64/kernel/kprobes-arm64.c is
meaningless. As we've done in x86, I think we can make it
arch/arm64/kernel/kprobes/decode-insn.{c,h}
I've changed the name to kprobe-decode-insn.[hc], or do you feel
strongly the three kprobes source files in arch/arm64/kernel need their
own subdirectory?
[..]
+
+/* Return:
+ * INSN_REJECTED If instruction is one not allowed to kprobe,
+ * INSN_GOOD If instruction is supported and uses instruction slot,
+ * INSN_GOOD_NO_SLOT If instruction is supported but doesn't use its slot.
Is there any chance to return INSN_GOOD_NO_SLOT?
Ah, that gets used later when simulation support is added. I've removed
this enum value from this commit and will add it to the later one.
Please no one complain about using an enum instead of a bool, it will
eventually have three possible values.
OK :)
[..]
+enum kprobe_insn __kprobes
+arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct arch_specific_insn *asi)
+{
+ enum kprobe_insn decoded;
+ kprobe_opcode_t insn = le32_to_cpu(*addr);
+ kprobe_opcode_t *scan_start = addr - 1;
+ kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
+#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
+ struct module *mod;
+#endif
+
+ if (addr >= (kprobe_opcode_t *)_text &&
+ scan_end < (kprobe_opcode_t *)_text)
+ scan_end = (kprobe_opcode_t *)_text;
+#if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
+ else {
+ preempt_disable();
+ mod = __module_address((unsigned long)addr);
+ if (mod && within_module_init((unsigned long)addr, mod) &&
+ !within_module_init((unsigned long)scan_end, mod))
+ scan_end = (kprobe_opcode_t *)mod->init_layout.base;
+ else if (mod && within_module_core((unsigned long)addr, mod) &&
+ !within_module_core((unsigned long)scan_end, mod))
+ scan_end = (kprobe_opcode_t *)mod->core_layout.base;
What happen if mod == NULL? it should be return error, isn't it?
No, it should be fine. It just means it didn't have to do either of the
extra checks to limit the end of the search through the code to the
boundary of one of the corresponding module text sections. It means the
instruction is in the regular kernel (non-module) text segment.
Ah, I see. It is OK then. :)
Thank you,
Thanks,
-dl