This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Kprobes- robust fault handling for i386


Anil,

On Fri, Feb 24, 2006 at 11:17:01AM -0800, Keshavamurthy, Anil S wrote:
> Prasanna,
> 	For better review comments, can you please split your patch
> into following, currently finding it hard to follow the kprobes states.
> 1) [PATCH]Fault handling due to calling pre_handler
> 2) [PATCH]Fault handling due to calling post_handler
> 3) [PATCH]Fault handling due to single_stepping
> 4) [PATCH]Fault handling due to single_stepping reentrant probe
> 
> Patches 3 and 4 above are required only to support user probes,
> so for now I think you can skip them.

For your convenience I have splitup the patches, please find
then below.
In general splitting of patches is a good idea, but here I think
splitting does not make much difference, since post_handler changes
are only few lines.  Correct me if I am wrong.
> 
> Also another suggesting, rename the kprobes states to something more
> meaning full. Say
> KPROBE_IN_PRE_HANDLER - This states indicates we are calling pre_handler
> KPROBE_IN_POST_HANDLER - This states indicates we are calling
> post_handler
> KPROBES_IN_SS - We are trying to single step
> KPROBES_IN_REENTER_SS - We are trying to single step reentrant probes 
> [...]
> 

Renaming states is a good idea, but we should do it independent of fault
handling. So how about doing it once we have robust fault handling in
place.

Thanks
Prasanna


This patch provides proper kprobes fault handling, if a user-specified
pre handlers tries to access user address space, through
copy_from_user(), get_user() etc. The user-specified fault handler
gets called only if the fault occurs wile executing user-specified
handlers. In such a case user-specified handler is allowed to fix it
first, later if the user-specifed fault handler does not fix it, we
try to fix it by calling fix_exception(). Also we set the "FAULTED"
flags if user-specified pre handler faults, so that corresponding
user-specified post_handler can be skipped. The user-specified handler
will not be called if the fault happens when single stepping the
original instruction, instead we reset the current probe and allow the
system page fault handler to fix it up.

Signed-off-by: Prasanna S Panchamukhi <prasanna@in.ibm.com>



 arch/i386/kernel/kprobes.c |   73 ++++++++++++++++++++++++++++++++++++++-------
 include/linux/kprobes.h    |   12 +++++++
 2 files changed, 75 insertions(+), 10 deletions(-)

diff -puN arch/i386/kernel/kprobes.c~kprobes-i386-pagefault-handling arch/i386/kernel/kprobes.c
--- linux-2.6.16-rc4-mm2/arch/i386/kernel/kprobes.c~kprobes-i386-pagefault-handling	2006-02-27 13:58:41.000000000 +0530
+++ linux-2.6.16-rc4-mm2-prasanna/arch/i386/kernel/kprobes.c	2006-02-27 14:08:45.000000000 +0530
@@ -35,6 +35,7 @@
 #include <asm/cacheflush.h>
 #include <asm/kdebug.h>
 #include <asm/desc.h>
+#include <asm/uaccess.h>
 
 void jprobe_return_end(void);
 
@@ -232,8 +233,9 @@ static int __kprobes kprobe_handler(stru
 	if (kprobe_running()) {
 		p = get_kprobe(addr);
 		if (p) {
-			if (kcb->kprobe_status == KPROBE_HIT_SS &&
-				*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
+			if (((kcb->kprobe_status == KPROBE_HIT_SS) ||
+				(kcb->kprobe_status == KPROBE_HIT_FAULT_SS)) &&
+				(*p->ainsn.insn == BREAKPOINT_INSTRUCTION)) {
 				regs->eflags &= ~TF_MASK;
 				regs->eflags |= kcb->kprobe_saved_eflags;
 				goto no_kprobe;
@@ -320,7 +322,10 @@ static int __kprobes kprobe_handler(stru
 
 ss_probe:
 	prepare_singlestep(p, regs);
-	kcb->kprobe_status = KPROBE_HIT_SS;
+	if (kcb->kprobe_status != KPROBE_HIT_FAULT)
+		kcb->kprobe_status = KPROBE_HIT_SS;
+	else
+		kcb->kprobe_status = KPROBE_HIT_FAULT_SS;
 	return 1;
 
 no_kprobe:
@@ -554,15 +559,63 @@ static inline int kprobe_fault_handler(s
 	struct kprobe *cur = kprobe_running();
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
-	if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
-		return 1;
-
-	if (kcb->kprobe_status & KPROBE_HIT_SS) {
-		resume_execution(cur, regs, kcb);
+	switch(kcb->kprobe_status) {
+	case KPROBE_HIT_SS:
+	case KPROBE_REENTER:
+	case KPROBE_HIT_FAULT_SS:
+		/*
+		 * We are here because the instruction being single
+		 * stepped caused a page fault. We reset the current
+		 * kprobe and the eip points back to the probe address
+		 * and allow the page fault handler to continue as a
+		 * normal page fault.
+		 */
+		regs->eip = (unsigned long)cur->addr;
 		regs->eflags |= kcb->kprobe_old_eflags;
-
-		reset_current_kprobe();
+		if (kcb->kprobe_status == KPROBE_REENTER)
+			restore_previous_kprobe(kcb);
+		else
+			reset_current_kprobe();
 		preempt_enable_no_resched();
+		break;
+	case KPROBE_HIT_ACTIVE:
+		/*
+		 *  We set the status as "FAULTED", so that subsequent
+		 *  user specified post handler can be avoided.
+		 */
+		kcb->kprobe_status = KPROBE_HIT_FAULT;
+		/*fixup the exception*/
+		/*
+		 * We increment the nmissed count for accounting,
+		 * we can also use npre/npostfault count for accouting
+		 * these specific fault cases.
+		 */
+		kprobes_inc_nmissed_count(cur);
+
+		/*
+		 * We come here because instructions in the pre/post
+		 * handler caused the page_fault, this could happen
+		 * if handler tries to access user space by
+		 * copy_from_user(), get_user() etc. Let the
+		 * user-specified handler try to fix it first.
+		 */
+		if (cur->fault_handler && cur->fault_handler(cur, regs, trapnr))
+			return 1;
+
+		/*
+		 * In case the user-specified fault handler returned
+		 * zero, try to fix up.
+		 */
+		if (fixup_exception(regs))
+			return 1;
+
+		/*
+		 * fixup_exception() could not handle it,
+		 * Let do_page_fault() fix it.
+		 */
+		break;
+	default:
+		break;
 	}
 	return 0;
 }
diff -puN include/linux/kprobes.h~kprobes-i386-pagefault-handling include/linux/kprobes.h
--- linux-2.6.16-rc4-mm2/include/linux/kprobes.h~kprobes-i386-pagefault-handling	2006-02-27 13:58:41.000000000 +0530
+++ linux-2.6.16-rc4-mm2-prasanna/include/linux/kprobes.h	2006-02-27 13:58:42.000000000 +0530
@@ -46,6 +46,18 @@
 #define KPROBE_HIT_SS		0x00000002
 #define KPROBE_REENTER		0x00000004
 #define KPROBE_HIT_SSDONE	0x00000008
+/*
+ * When set, signifies that the fault happened
+ * while in the user-specified pre_handler.
+ */
+#define KPROBE_HIT_FAULT	0x00000010
+/*
+ * When set, signifies that the faulted user-specified
+ * pre_handler has been executed, now allow single
+ * stepping of original instruction and dont execute
+ * the post_handler after single stepping.
+ */
+#define KPROBE_HIT_FAULT_SS	0x00000020
 
 /* Attach to insert probes on any functions which should be ignored*/
 #define __kprobes	__attribute__((__section__(".kprobes.text")))

_
-- 
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Email: prasanna@in.ibm.com
Ph: 91-80-51776329


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]