diff for duplicates of <5551F693.8050100@redhat.com> diff --git a/a/1.txt b/N1/1.txt index e998bf2..577abb0 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -70,10 +70,3 @@ Hi Dave, In some of the previous diagnostic output it looked like things would go wrong in the entry.S when the D bit was cleared and the debug interrupts were unmasksed. I wonder if some of the issue might be due to the starting the kprobe for the trampoline, but leaving things in an odd state when another set of krpobe/kretporbes are hit when the trampoline is running. As Dave mentioned the proposed trampoline patch avoids using a kprobe in the trampoline and directly calls the trampoline handler. Attached is the current version of the patch which was able to run the systemtap testsuite. Systemtap does exercise the kprobe/kretprobe infrastructure, but it would be good to have additional raw kprobe tests to check that kprobe reentry works as expected. -Will Cohen --------------- next part -------------- -A non-text attachment was scrubbed... -Name: avoid_bkpt_tramp_v2.diff -Type: text/x-patch -Size: 4738 bytes -Desc: not available -URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150512/098399ee/attachment.bin> diff --git a/N1/2.hdr b/N1/2.hdr new file mode 100644 index 0000000..b7d249c --- /dev/null +++ b/N1/2.hdr @@ -0,0 +1,5 @@ +Content-Type: text/x-patch; + name="avoid_bkpt_tramp_v2.diff" +Content-Transfer-Encoding: 7bit +Content-Disposition: attachment; + filename="avoid_bkpt_tramp_v2.diff" diff --git a/N1/2.txt b/N1/2.txt new file mode 100644 index 0000000..defb9f2 --- /dev/null +++ b/N1/2.txt @@ -0,0 +1,136 @@ +diff --git a/arch/arm64/kernel/kprobes-arm64.h b/arch/arm64/kernel/kprobes-arm64.h +index ff8a55f..46dab24 100644 +--- a/arch/arm64/kernel/kprobes-arm64.h ++++ b/arch/arm64/kernel/kprobes-arm64.h +@@ -27,4 +27,47 @@ extern kprobes_pstate_check_t * const kprobe_condition_checks[16]; + enum kprobe_insn __kprobes + arm_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi); + ++#define SAVE_REGS_STRING\ ++ " stp x0, x1, [sp, #16 * 0]\n" \ ++ " stp x2, x3, [sp, #16 * 1]\n" \ ++ " stp x4, x5, [sp, #16 * 2]\n" \ ++ " stp x6, x7, [sp, #16 * 3]\n" \ ++ " stp x8, x9, [sp, #16 * 4]\n" \ ++ " stp x10, x11, [sp, #16 * 5]\n" \ ++ " stp x12, x13, [sp, #16 * 6]\n" \ ++ " stp x14, x15, [sp, #16 * 7]\n" \ ++ " stp x16, x17, [sp, #16 * 8]\n" \ ++ " stp x18, x19, [sp, #16 * 9]\n" \ ++ " stp x20, x21, [sp, #16 * 10]\n" \ ++ " stp x22, x23, [sp, #16 * 11]\n" \ ++ " stp x24, x25, [sp, #16 * 12]\n" \ ++ " stp x26, x27, [sp, #16 * 13]\n" \ ++ " stp x28, x29, [sp, #16 * 14]\n" \ ++ " str x30, [sp, #16 * 15]\n" \ ++ " mrs x0, nzcv\n" \ ++ " str x0, [sp, #8 * 33]\n" ++ ++ ++#define RESTORE_REGS_STRING\ ++ " ldr x0, [sp, #8 * 33]\n" \ ++ " msr nzcv, x0\n" \ ++ " ldp x0, x1, [sp, #16 * 0]\n" \ ++ " ldp x2, x3, [sp, #16 * 1]\n" \ ++ " ldp x4, x5, [sp, #16 * 2]\n" \ ++ " ldp x6, x7, [sp, #16 * 3]\n" \ ++ " ldp x8, x9, [sp, #16 * 4]\n" \ ++ " ldp x10, x11, [sp, #16 * 5]\n" \ ++ " ldp x12, x13, [sp, #16 * 6]\n" \ ++ " ldp x14, x15, [sp, #16 * 7]\n" \ ++ " ldp x16, x17, [sp, #16 * 8]\n" \ ++ " ldp x18, x19, [sp, #16 * 9]\n" \ ++ " ldp x20, x21, [sp, #16 * 10]\n" \ ++ " ldp x22, x23, [sp, #16 * 11]\n" \ ++ " ldp x24, x25, [sp, #16 * 12]\n" \ ++ " ldp x26, x27, [sp, #16 * 13]\n" \ ++ " ldp x28, x29, [sp, #16 * 14]\n" \ ++ " ldr x30, [sp, #16 * 15]\n" \ ++ ++ ++ + #endif /* _ARM_KERNEL_KPROBES_ARM64_H */ +diff --git a/arch/arm64/kernel/kprobes.c b/arch/arm64/kernel/kprobes.c +index d2aa4bc..af55e37 100644 +--- a/arch/arm64/kernel/kprobes.c ++++ b/arch/arm64/kernel/kprobes.c +@@ -565,32 +565,27 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs) + } + + /* +- * Kretprobes: kernel return probes handling +- * +- * AArch64 mode does not support popping the PC value from the +- * stack like on ARM 32-bit (ldmia {..,pc}), so atleast one +- * register need to be used to achieve branching/return. +- * It means return probes cannot return back to the original +- * return address directly without modifying the register context. +- * +- * So like other architectures, we prepare a global routine +- * with NOPs, which serve as trampoline address that hack away the +- * function return, with the exact register context. +- * Placing a kprobe on trampoline routine entry will trap again to +- * execute return probe handlers and restore original return address +- * in ELR_EL1, this way saved pt_regs still hold the original +- * register values to be carried back to the caller. ++ * When a retprobed function returns, this code saves registers and ++ * calls trampoline_handler() runs, which calls the kretprobe's handler. + */ +-static void __used kretprobe_trampoline_holder(void) ++static void __used __kprobes kretprobe_trampoline_holder(void) + { + asm volatile (".global kretprobe_trampoline\n" + "kretprobe_trampoline:\n" +- "NOP\n\t" +- "NOP\n\t"); ++ "sub sp, sp, %0\n" ++ SAVE_REGS_STRING ++ "mov x0, sp\n" ++ "bl trampoline_probe_handler\n" ++ /* Replace trampoline address in lr with actual ++ orig_ret_addr return address. */ ++ "str x0, [sp, #16 * 15]\n" ++ RESTORE_REGS_STRING ++ "add sp, sp, %0\n" ++ "ret\n" ++ : : "I"(sizeof(struct pt_regs)) : "memory"); + } + +-static int __kprobes +-trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) ++static void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs) + { + struct kretprobe_instance *ri = NULL; + struct hlist_head *head, empty_rp; +@@ -651,7 +646,7 @@ trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs) + } + + /* return 1 so that post handlers not called */ +- return 1; ++ return (void *) orig_ret_addr; + } + + void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, +@@ -663,18 +658,12 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, + regs->regs[30] = (long)&kretprobe_trampoline; + } + +-static struct kprobe trampoline = { +- .addr = (kprobe_opcode_t *) &kretprobe_trampoline, +- .pre_handler = trampoline_probe_handler +-}; +- +-int __kprobes arch_trampoline_kprobe(struct kprobe *p) ++int __init arch_init_kprobes(void) + { +- return p->addr == (kprobe_opcode_t *) &kretprobe_trampoline; ++ return 0; + } + +-int __init arch_init_kprobes(void) ++int arch_trampoline_kprobe(struct kprobe *p) + { +- /* register trampoline for kret probe */ +- return register_kprobe(&trampoline); ++ return 0; + } diff --git a/a/content_digest b/N1/content_digest index f340429..85d7bf9 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -7,11 +7,23 @@ "ref\0554851CB.2010909@linaro.org\0" "ref\020150505154830.GI1550@arm.com\0" "ref\055519583.9020507@linaro.org\0" - "From\0wcohen@redhat.com (William Cohen)\0" - "Subject\0[PATCH v6 0/6] arm64: Add kernel probes (kprobes) support\0" + "From\0William Cohen <wcohen@redhat.com>\0" + "Subject\0Re: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support\0" "Date\0Tue, 12 May 2015 08:48:19 -0400\0" - "To\0linux-arm-kernel@lists.infradead.org\0" - "\00:1\0" + "To\0David Long <dave.long@linaro.org>" + " Will Deacon <will.deacon@arm.com>\0" + "Cc\0Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>" + linux-arm-kernel@lists.infradead.org <linux-arm-kernel@lists.infradead.org> + Russell King <linux@arm.linux.org.uk> + sandeepa.s.prabhu@gmail.com <sandeepa.s.prabhu@gmail.com> + Steve Capper <steve.capper@linaro.org> + Catalin Marinas <Catalin.Marinas@arm.com> + Jon Medhurst (Tixy) <tixy@linaro.org> + Ananth N Mavinakayanahalli <ananth@in.ibm.com> + Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com> + davem@davemloft.net <davem@davemloft.net> + " linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>\0" + "\01:1\0" "b\0" "On 05/12/2015 01:54 AM, David Long wrote:\n" "> On 05/05/15 11:48, Will Deacon wrote:\n" @@ -84,13 +96,145 @@ "\n" "In some of the previous diagnostic output it looked like things would go wrong in the entry.S when the D bit was cleared and the debug interrupts were unmasksed. I wonder if some of the issue might be due to the starting the kprobe for the trampoline, but leaving things in an odd state when another set of krpobe/kretporbes are hit when the trampoline is running. As Dave mentioned the proposed trampoline patch avoids using a kprobe in the trampoline and directly calls the trampoline handler. Attached is the current version of the patch which was able to run the systemtap testsuite. Systemtap does exercise the kprobe/kretprobe infrastructure, but it would be good to have additional raw kprobe tests to check that kprobe reentry works as expected.\n" "\n" - "-Will Cohen\n" - "-------------- next part --------------\n" - "A non-text attachment was scrubbed...\n" - "Name: avoid_bkpt_tramp_v2.diff\n" - "Type: text/x-patch\n" - "Size: 4738 bytes\n" - "Desc: not available\n" - URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150512/098399ee/attachment.bin> + -Will Cohen + "\01:2\0" + "fn\0avoid_bkpt_tramp_v2.diff\0" + "b\0" + "diff --git a/arch/arm64/kernel/kprobes-arm64.h b/arch/arm64/kernel/kprobes-arm64.h\n" + "index ff8a55f..46dab24 100644\n" + "--- a/arch/arm64/kernel/kprobes-arm64.h\n" + "+++ b/arch/arm64/kernel/kprobes-arm64.h\n" + "@@ -27,4 +27,47 @@ extern kprobes_pstate_check_t * const kprobe_condition_checks[16];\n" + " enum kprobe_insn __kprobes\n" + " arm_kprobe_decode_insn(kprobe_opcode_t insn, struct arch_specific_insn *asi);\n" + " \n" + "+#define SAVE_REGS_STRING\\\n" + "+\t\"\tstp x0, x1, [sp, #16 * 0]\\n\"\t\\\n" + "+\t\"\tstp x2, x3, [sp, #16 * 1]\\n\"\t\\\n" + "+\t\"\tstp x4, x5, [sp, #16 * 2]\\n\"\t\\\n" + "+\t\"\tstp x6, x7, [sp, #16 * 3]\\n\"\t\\\n" + "+\t\"\tstp x8, x9, [sp, #16 * 4]\\n\"\t\\\n" + "+\t\"\tstp x10, x11, [sp, #16 * 5]\\n\"\t\\\n" + "+\t\"\tstp x12, x13, [sp, #16 * 6]\\n\"\t\\\n" + "+\t\"\tstp x14, x15, [sp, #16 * 7]\\n\"\t\\\n" + "+\t\"\tstp x16, x17, [sp, #16 * 8]\\n\"\t\\\n" + "+\t\"\tstp x18, x19, [sp, #16 * 9]\\n\"\t\\\n" + "+\t\"\tstp x20, x21, [sp, #16 * 10]\\n\"\t\\\n" + "+\t\"\tstp x22, x23, [sp, #16 * 11]\\n\"\t\\\n" + "+\t\"\tstp x24, x25, [sp, #16 * 12]\\n\"\t\\\n" + "+\t\"\tstp x26, x27, [sp, #16 * 13]\\n\"\t\\\n" + "+\t\"\tstp x28, x29, [sp, #16 * 14]\\n\"\t\\\n" + "+\t\"\tstr x30, [sp, #16 * 15]\\n\" \\\n" + "+\t\"\tmrs x0, nzcv\\n\"\t\t\t\\\n" + "+\t\"\tstr x0, [sp, #8 * 33]\\n\"\n" + "+\t\n" + "+\n" + "+#define RESTORE_REGS_STRING\\\n" + "+\t\"\tldr x0, [sp, #8 * 33]\\n\"\t\\\n" + "+\t\"\tmsr nzcv, x0\\n\"\t\t\t\\\n" + "+\t\"\tldp x0, x1, [sp, #16 * 0]\\n\"\t\\\n" + "+\t\"\tldp x2, x3, [sp, #16 * 1]\\n\"\t\\\n" + "+\t\"\tldp x4, x5, [sp, #16 * 2]\\n\"\t\\\n" + "+\t\"\tldp x6, x7, [sp, #16 * 3]\\n\"\t\\\n" + "+\t\"\tldp x8, x9, [sp, #16 * 4]\\n\"\t\\\n" + "+\t\"\tldp x10, x11, [sp, #16 * 5]\\n\"\t\\\n" + "+\t\"\tldp x12, x13, [sp, #16 * 6]\\n\"\t\\\n" + "+\t\"\tldp x14, x15, [sp, #16 * 7]\\n\"\t\\\n" + "+\t\"\tldp x16, x17, [sp, #16 * 8]\\n\"\t\\\n" + "+\t\"\tldp x18, x19, [sp, #16 * 9]\\n\"\t\\\n" + "+\t\"\tldp x20, x21, [sp, #16 * 10]\\n\"\t\\\n" + "+\t\"\tldp x22, x23, [sp, #16 * 11]\\n\"\t\\\n" + "+\t\"\tldp x24, x25, [sp, #16 * 12]\\n\"\t\\\n" + "+\t\"\tldp x26, x27, [sp, #16 * 13]\\n\"\t\\\n" + "+\t\"\tldp x28, x29, [sp, #16 * 14]\\n\"\t\\\n" + "+\t\"\tldr x30, [sp, #16 * 15]\\n\"\t\\\n" + "+\n" + "+\n" + "+\n" + " #endif /* _ARM_KERNEL_KPROBES_ARM64_H */\n" + "diff --git a/arch/arm64/kernel/kprobes.c b/arch/arm64/kernel/kprobes.c\n" + "index d2aa4bc..af55e37 100644\n" + "--- a/arch/arm64/kernel/kprobes.c\n" + "+++ b/arch/arm64/kernel/kprobes.c\n" + "@@ -565,32 +565,27 @@ int __kprobes longjmp_break_handler(struct kprobe *p, struct pt_regs *regs)\n" + " }\n" + " \n" + " /*\n" + "- * Kretprobes: kernel return probes handling\n" + "- *\n" + "- * AArch64 mode does not support popping the PC value from the\n" + "- * stack like on ARM 32-bit (ldmia {..,pc}), so atleast one\n" + "- * register need to be used to achieve branching/return.\n" + "- * It means return probes cannot return back to the original\n" + "- * return address directly without modifying the register context.\n" + "- *\n" + "- * So like other architectures, we prepare a global routine\n" + "- * with NOPs, which serve as trampoline address that hack away the\n" + "- * function return, with the exact register context.\n" + "- * Placing a kprobe on trampoline routine entry will trap again to\n" + "- * execute return probe handlers and restore original return address\n" + "- * in ELR_EL1, this way saved pt_regs still hold the original\n" + "- * register values to be carried back to the caller.\n" + "+ * When a retprobed function returns, this code saves registers and\n" + "+ * calls trampoline_handler() runs, which calls the kretprobe's handler.\n" + " */\n" + "-static void __used kretprobe_trampoline_holder(void)\n" + "+static void __used __kprobes kretprobe_trampoline_holder(void)\n" + " {\n" + " \tasm volatile (\".global kretprobe_trampoline\\n\"\n" + " \t\t\t\"kretprobe_trampoline:\\n\"\n" + "-\t\t\t\"NOP\\n\\t\"\n" + "-\t\t\t\"NOP\\n\\t\");\n" + "+\t\t \"sub sp, sp, %0\\n\"\n" + "+\t\t\tSAVE_REGS_STRING\n" + "+\t\t\t\"mov x0, sp\\n\"\n" + "+\t\t\t\"bl trampoline_probe_handler\\n\"\n" + "+\t\t\t/* Replace trampoline address in lr with actual\n" + "+\t\t\t orig_ret_addr return address. */\n" + "+\t\t\t\"str x0, [sp, #16 * 15]\\n\"\n" + "+\t\t\tRESTORE_REGS_STRING\n" + "+\t\t \"add sp, sp, %0\\n\"\n" + "+\t\t\t\"ret\\n\"\n" + "+\t\t : : \"I\"(sizeof(struct pt_regs)) : \"memory\");\n" + " }\n" + " \n" + "-static int __kprobes\n" + "-trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs)\n" + "+static void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)\n" + " {\n" + " \tstruct kretprobe_instance *ri = NULL;\n" + " \tstruct hlist_head *head, empty_rp;\n" + "@@ -651,7 +646,7 @@ trampoline_probe_handler(struct kprobe *p, struct pt_regs *regs)\n" + " \t}\n" + " \n" + " \t/* return 1 so that post handlers not called */\n" + "-\treturn 1;\n" + "+\treturn (void *) orig_ret_addr;\n" + " }\n" + " \n" + " void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,\n" + "@@ -663,18 +658,12 @@ void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri,\n" + " \tregs->regs[30] = (long)&kretprobe_trampoline;\n" + " }\n" + " \n" + "-static struct kprobe trampoline = {\n" + "-\t.addr = (kprobe_opcode_t *) &kretprobe_trampoline,\n" + "-\t.pre_handler = trampoline_probe_handler\n" + "-};\n" + "-\n" + "-int __kprobes arch_trampoline_kprobe(struct kprobe *p)\n" + "+int __init arch_init_kprobes(void)\n" + " {\n" + "-\treturn p->addr == (kprobe_opcode_t *) &kretprobe_trampoline;\n" + "+\treturn 0;\n" + " }\n" + " \n" + "-int __init arch_init_kprobes(void)\n" + "+int arch_trampoline_kprobe(struct kprobe *p)\n" + " {\n" + "-\t/* register trampoline for kret probe */\n" + "-\treturn register_kprobe(&trampoline);\n" + "+\treturn 0;\n" + } -9bd6f426f4b7e70bd31f225a088f3e5a6a8f20f26eba4ea3c83d2a4ac53b4f11 +b3a79d3406db7ac287c1c66536c6254a2718c37b8ac25011691d55cfb45731f4
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.