Re: [PATCH v6 0/6] arm64: Add kernel probes (kprobes) support

[William Cohen]

On 04/21/2015 07:42 AM, Masami Hiramatsu wrote:
> (2015/04/21 5:19), David Long wrote:
>> From: "David A. Long" <dave.long@xxxxxxxxxx>
>>
>> This patchset is heavily based on Sandeepa Prabhu's ARM v8 kprobes patches,
>> first seen in October 2013. This version attempts to address concerns raised by
>> reviewers and also fixes problems discovered during testing.
>>
>> This patchset adds support for kernel probes(kprobes), jump probes(jprobes)
>> and return probes(kretprobes) support for ARM64.
>>
>> The kprobes mechanism makes use of software breakpoint and single stepping
>> support available in the ARM v8 kernel.
>>
> [...]
>> Changes since v5 include:
>>
>> 1) Replaced installation of breakpoint hook with direct call from the
>> handlers in debug-monitors.c, as requested.
>> 2) Reject probing of instructions that read the interrupt mask, in
>> addition to instructions that set it.
>> 3) Cleaned up comments describing usage of Debug Mask.
>> 4) Added KPROBE_REENTER case in reenter_kprobe.
>> 5) Corrected the ifdef'd definitions for notify_page_fault() to be
>> consistent when KPROBES is not configed.
>> 6) Changed "cpsr" to "pstate" for HAVE_REGS_AND_STACK_ACCESS_API feature.
>> 7) Added back in missing new files in previous patch.
>> 8) Changed two instances of pr_warning() to pr_warn().
> 
> Looks OK to me:)
> BTW, have you tried to build and test this with CONFIG_KPROBE_EVENT?
> If so, you can also test it by tools/testing/selftests/ftrace/ftracetest.
> 
>> Note that there seems to be at least a potential issue with kprobes
>> on multiple (possibly all) platforms having to do with use of kfree
>> inside of the kretprobes trampoline handler.  This has manifested
>> occasionally in systemtap testing on arm64.  There does not appear to
>> be an simple solution to the problem.
> 
> No, trampoline handler must call recycle_rp_inst() instead of kfree
> to return kretprobe instance to the pool. Hmm, I should look into it.
> 
> Thank you,
> 

Hi,

I have noticed when running the systemtap testsuite even with this newest revision of the arm64 kprobe patches the system will start spewing the following message:

Unexpected kernel single-step exception at EL1

That is triggered by the functioncallcount.stp test in the systemtap examples.  The test is instrumenting many function calls and returns in the memory management code.  It appears that the problem is triggered during the kfree call/return towards the end of trampoline_probe_handler.  The single_step_handler function in debug-monitors.c calls kprobe_single_step_handler which indicates that the breakpoint is not a kprobe breakpoint.  Thus, single_step_handler starts flagging the breakpoint with the above message.

It would be good if the warning message included and address to be a bit more informative.  I put in some addition code (the attached patch) to print out register information to diagnose what was going on.

Other architecture do use kfree in the kretprobe trampoline handlers but do not seem to encounter this problem. 

-Will
diff --git a/arch/arm64/kernel/debug-monitors.c b/arch/arm64/kernel/debug-monitors.c
index dae7bb4..ec5a1b2 100644
--- a/arch/arm64/kernel/debug-monitors.c
+++ b/arch/arm64/kernel/debug-monitors.c
@@ -262,6 +262,19 @@ static int single_step_handler(unsigned long addr, unsigned int esr,
 
 		if (!handler_found) {
 			pr_warning("Unexpected kernel single-step exception at EL1\n");
+			{
+		  	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+			pr_warning("kcb->ss_ctx.ss_status = %ld\n",
+				   kcb->ss_ctx.ss_status);
+			printk("kcb->ss_ctx.match_addr = %lx ",
+				     kcb->ss_ctx.match_addr);
+			print_symbol("%s\n", kcb->ss_ctx.match_addr);
+			printk("instruction_pointer(regs) = %lx ",
+				   instruction_pointer(regs));
+			print_symbol("%s\n", instruction_pointer(regs));
+			show_regs(regs);
+			BUG();
+			}
 			/*
 			 * Re-enable stepping since we know that we will be
 			 * returning to regs.