[RFC][PATCH] Kprobes robust fault handling for i386

[RFC][PATCH] Kprobes robust fault handling for i386

[Prasanna S Panchamukhi]

Please find the patch for robust fault handling of kprobes for
i386 architecture. This patch requires kgdb-setjmp/longjmp
routines defined in kgdb-jmp.c file. If this approach
looks ok, we can ask kgdb maintainer to push setjmp/longjmp
routines to mainline.

Please let me know if you have any issues.

Thanks
Prasanna

If a user-specified pre/post handler generates a fault say, due to
access to user address space, through copy_from_user(), get_user() etc it
must be fixed. Kprobes handles this by calling fixup_exception(). If
fixup_exception() does not succeed, then the faulty handler must not be
allowed to continue further. This patch provides a simple mechanism
to save and restore the state in such cases by aborting the faulted
user-specified handler. Sufficient registers must be saved so that kprobes
can recover from such faults. This patch uses the kgdb_fault_setjmp() and
kgdb_fault_longjmp() routines which save the minimum set of registers
required to recover from faults.  The resgisters are saved and restored
from the per-probe kprobe_ctlblk structure. If there are multiple handlers
registered for a probepoint, then only the faulted user-specified handler is
aborted, the rest of the user-specified handlers in the aggregate probe list
get executed.

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


 arch/i386/kernel/Makefile  |    5 ++-
 arch/i386/kernel/kprobes.c |   60 ++++++++++++++++++++++++++++++++++++++---
 include/asm-i386/kprobes.h |    8 +++++
 kernel/kprobes.c           |   65 +++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 129 insertions(+), 9 deletions(-)

diff -puN include/asm-i386/kprobes.h~kprobes-robust-fault-hanlding-for-i386 include/asm-i386/kprobes.h
--- linux-2.6.17-rc1-mm2/include/asm-i386/kprobes.h~kprobes-robust-fault-hanlding-for-i386	2006-04-24 12:04:07.000000000 +0530
+++ linux-2.6.17-rc1-mm2-prasanna/include/asm-i386/kprobes.h	2006-04-24 12:04:08.000000000 +0530
@@ -44,6 +44,8 @@ typedef u8 kprobe_opcode_t;
 
 #define JPROBE_ENTRY(pentry)	(kprobe_opcode_t *)pentry
 #define ARCH_SUPPORTS_KRETPROBES
+#define ARCH_SUPPORTS_FAULT_HANDLING
+#define NUMCRITREG	6
 
 void arch_remove_kprobe(struct kprobe *p);
 void kretprobe_trampoline(void);
@@ -71,6 +73,7 @@ struct kprobe_ctlblk {
 	unsigned long kprobe_status;
 	unsigned long kprobe_old_eflags;
 	unsigned long kprobe_saved_eflags;
+	unsigned long kprobe_fault_jmp_regs[NUMCRITREG];
 	long *jprobe_saved_esp;
 	struct pt_regs jprobe_saved_regs;
 	kprobe_opcode_t jprobes_stack[MAX_STACK_SIZE];
@@ -88,4 +91,9 @@ static inline void restore_interrupts(st
 
 extern int kprobe_exceptions_notify(struct notifier_block *self,
 				    unsigned long val, void *data);
+extern int asmlinkage kgdb_fault_setjmp(unsigned long *);
+extern int asmlinkage kgdb_fault_longjmp(unsigned long *);
+extern int aggr_pre_handler(struct kprobe *p, struct pt_regs *regs);
+extern void aggr_post_handler(struct kprobe *p, struct pt_regs *regs,
+							unsigned long flags);
 #endif				/* _ASM_KPROBES_H */
diff -puN kernel/kprobes.c~kprobes-robust-fault-hanlding-for-i386 kernel/kprobes.c
--- linux-2.6.17-rc1-mm2/kernel/kprobes.c~kprobes-robust-fault-hanlding-for-i386	2006-04-24 12:04:07.000000000 +0530
+++ linux-2.6.17-rc1-mm2-prasanna/kernel/kprobes.c	2006-04-25 12:11:15.000000000 +0530
@@ -189,9 +189,40 @@ struct kprobe __kprobes *get_kprobe(void
  * Aggregate handlers for multiple kprobes support - these handlers
  * take care of invoking the individual kprobe handlers on p->list
  */
-static int __kprobes aggr_pre_handler(struct kprobe *p, struct pt_regs *regs)
+int __kprobes aggr_pre_handler(struct kprobe *p, struct pt_regs *regs)
 {
 	struct kprobe *kp;
+#ifdef ARCH_SUPPORTS_FAULT_HANDLING
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+	int skipped_faulty_handler;
+	struct kprobe *faulty_kprobe;
+	/*
+	 * Save enough registers before calling the user-specified
+	 * handlers, so that if a user-specified handler generates a
+	 * fault, it could be recovered to a sane state. Also since
+	 * mutiple handlers are registered for this kprobe, only the
+	 * faulted user-specified handler must be prevented from
+	 * further execution, allow other handler in the aggregate
+	 * kprobes list to execute.
+	 */
+	if ((kgdb_fault_setjmp(kcb->kprobe_fault_jmp_regs)) != 0) {
+		skipped_faulty_handler = 0;
+		faulty_kprobe = __get_cpu_var(kprobe_instance);
+		list_for_each_entry_rcu(kp, &p->list, list) {
+			if (!skipped_faulty_handler && (kp == faulty_kprobe))
+				skipped_faulty_handler = 1;
+			else if (skipped_faulty_handler == 1) {
+				if (kp->pre_handler) {
+					set_kprobe_instance(kp);
+					if (kp->pre_handler(kp, regs))
+						return 1;
+				}
+				reset_kprobe_instance();
+			}
+		}
+	return  0;
+	}
+#endif
 
 	list_for_each_entry_rcu(kp, &p->list, list) {
 		if (kp->pre_handler) {
@@ -204,10 +235,40 @@ static int __kprobes aggr_pre_handler(st
 	return 0;
 }
 
-static void __kprobes aggr_post_handler(struct kprobe *p, struct pt_regs *regs,
+void __kprobes aggr_post_handler(struct kprobe *p, struct pt_regs *regs,
 					unsigned long flags)
 {
 	struct kprobe *kp;
+#ifdef ARCH_SUPPORTS_FAULT_HANDLING
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+	int skipped_faulty_handler;
+	struct kprobe *faulty_kprobe;
+	/*
+	 * Save enough registers before calling the user-specified
+	 * handlers, so that if a user-specified handler generates a
+	 * fault, it could be recovered to a sane state. Also since
+	 * mutiple handlers are registered for this kprobe, only the
+	 * faulted user-specified handler must be prevented from
+	 * further execution, allow other handler in the aggregate
+	 * kprobes list to execute.
+	 */
+	if ((kgdb_fault_setjmp(kcb->kprobe_fault_jmp_regs)) != 0) {
+		skipped_faulty_handler = 0;
+		faulty_kprobe = __get_cpu_var(kprobe_instance);
+		list_for_each_entry_rcu(kp, &p->list, list) {
+			if (!skipped_faulty_handler && (kp == faulty_kprobe))
+				skipped_faulty_handler = 1;
+			else if (skipped_faulty_handler == 1) {
+				if (kp->post_handler) {
+					set_kprobe_instance(kp);
+					kp->post_handler(kp, regs, flags);
+					reset_kprobe_instance();
+				}
+			}
+		}
+	return;
+	}
+#endif
 
 	list_for_each_entry_rcu(kp, &p->list, list) {
 		if (kp->post_handler) {
diff -puN arch/i386/kernel/kprobes.c~kprobes-robust-fault-hanlding-for-i386 arch/i386/kernel/kprobes.c
--- linux-2.6.17-rc1-mm2/arch/i386/kernel/kprobes.c~kprobes-robust-fault-hanlding-for-i386	2006-04-24 12:04:07.000000000 +0530
+++ linux-2.6.17-rc1-mm2-prasanna/arch/i386/kernel/kprobes.c	2006-04-25 11:45:06.000000000 +0530
@@ -290,9 +290,26 @@ static int __kprobes kprobe_handler(stru
 	set_current_kprobe(p, regs, kcb);
 	kcb->kprobe_status = KPROBE_HIT_ACTIVE;
 
-	if (p->pre_handler && p->pre_handler(p, regs))
+	if (p->pre_handler) {
+		/*
+		 * If a user-specified handler faults, and the fault isn't
+		 * handled successfully, then the handler must be prevented
+		 * from continuing. Before executing the user-specified
+		 * handler, save enough registers in the kprobe_ctlblk
+		 * structure so that kprobes can recover from faults.
+		 * kgdb_fault_setjmp() saves enough registers to recover
+		 * from such faults.
+		 */
+		if ((p->pre_handler != aggr_pre_handler) &&
+			((kgdb_fault_setjmp(kcb->kprobe_fault_jmp_regs)) != 0))
+			 /* Uhhh! came here because of kgdb_fault_longjmp() */
+			goto boost_probe;
+
+		if (p->pre_handler(p, regs))
 		/* handler has already set things up, so skip ss setup */
-		return 1;
+			return 1;
+	}
+boost_probe:
 
 	if (p->ainsn.boostable == 1 &&
 #ifdef CONFIG_PREEMPT
@@ -320,6 +337,20 @@ no_kprobe:
 }
 
 /*
+ * This trampoline does a long jump, when a user-specified
+ * handler page faults, thereby restoring kprobes to a sane
+ * state.
+ */
+int trampoline_fault_handler(void)
+{
+	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
+
+	kgdb_fault_longjmp(kcb->kprobe_fault_jmp_regs);
+
+	return 0;
+}
+
+/*
  * For function-return probes, init_kprobes() establishes a probepoint
  * here. When a retprobed function returns, this probe is hit and
  * trampoline_probe_handler() runs, calling the kretprobe's handler.
@@ -517,9 +548,24 @@ static inline int post_kprobe_handler(st
 
 	if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) {
 		kcb->kprobe_status = KPROBE_HIT_SSDONE;
+		/*
+		 * If a user-specified handler faults, and the fault isn't
+		 * handled successfully, then the handler must be prevented
+		 * from continuing. Before executing the user-specified
+		 * handler, save enough registers in the kprobe_ctlblk
+		 * structure so that kprobes can recover from faults.
+		 * kgdb_fault_setjmp() saves enough registers to recover
+		 * from such faults.
+		 */
+		if ((cur->post_handler != aggr_post_handler) &&
+			((kgdb_fault_setjmp(kcb->kprobe_fault_jmp_regs)) != 0))
+			/* Uhhh! came here because of kgdb_fault_longjmp() */
+			goto resume_exec;
+
 		cur->post_handler(cur, regs, 0);
 	}
 
+resume_exec:
 	resume_execution(cur, regs, kcb);
 	regs->eflags |= kcb->kprobe_saved_eflags;
 
@@ -587,15 +633,19 @@ static inline int kprobe_fault_handler(s
 
 		/*
 		 * In case the user-specified fault handler returned
-		 * zero, try to fix up.
+		 * zero, we try to fix it.
 		 */
 		if (fixup_exception(regs))
 			return 1;
 
 		/*
-		 * fixup_exception() could not handle it,
-		 * Let do_page_fault() fix it.
+		 * In case if fixup_exception() does not fix it, then
+		 * we return to a trampoline_fault_handler(), that
+		 * does a long jump to the kprobes handler.
 		 */
+		regs->eip = (unsigned long)trampoline_fault_handler;
+		return 1;
+
 		break;
 	default:
 		break;
diff -puN arch/i386/kernel/Makefile~kprobes-robust-fault-hanlding-for-i386 arch/i386/kernel/Makefile
--- linux-2.6.17-rc1-mm2/arch/i386/kernel/Makefile~kprobes-robust-fault-hanlding-for-i386	2006-04-24 12:04:07.000000000 +0530
+++ linux-2.6.17-rc1-mm2-prasanna/arch/i386/kernel/Makefile	2006-04-24 12:04:48.000000000 +0530
@@ -7,7 +7,8 @@ extra-y := head.o init_task.o vmlinux.ld
 obj-y	:= process.o semaphore.o signal.o entry.o traps.o irq.o \
 		ptrace.o time.o ioport.o ldt.o setup.o i8259.o sys_i386.o \
 		pci-dma.o i386_ksyms.o i387.o bootflag.o \
-		quirks.o i8237.o topology.o alternative.o i8253.o tsc.o
+		quirks.o i8237.o topology.o alternative.o i8253.o tsc.o \
+		kgdb-jmp.o
 
 obj-y				+= cpu/
 obj-y				+= acpi/
@@ -36,7 +37,7 @@ obj-$(CONFIG_EFI) 		+= efi.o efi_stub.o
 obj-$(CONFIG_DOUBLEFAULT) 	+= doublefault.o
 obj-$(CONFIG_VM86)		+= vm86.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
-obj-$(CONFIG_KGDB)		+= kgdb.o kgdb-jmp.o
+obj-$(CONFIG_KGDB)		+= kgdb.o
 obj-$(CONFIG_HPET_TIMER) 	+= hpet.o
 
 EXTRA_AFLAGS   := -traditional

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

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

[Frank Ch. Eigler]

Prasanna S Panchamukhi <prasanna@in.ibm.com> writes:

> Please find the patch for robust fault handling of kprobes for i386
> architecture. [...]

This patch puts the setjmp into the kprobes layer, around the
invocation of the probe handler.  A longjmp due to a systemtap probe
problem would skip all the cleanup code (locking, error tracking,
context cleanup) that the probe handler boilerplate includes.  I had
previously imagined letting a probe handler include the setjmp
explicitly, so it can have its own cleanup code in systemtap-emitted
code.

This scheme would mean that the subject processor will not be able to
run any further systemtap probes, and probes on other processors will
timeout acquiring locks on any shared variables.  This would cause the
systemtap session to come to a prompt close due to MAXSKIPPED.

There are problems associated with overall session shutdown.  The
module removal code blocks until all running probe handler finish.
But if it has no way of knowing that one was "stuck", it could wait a
mighty long time.  This would likely have to be solved for the
proposed scheme to be acceptable.

- FChE

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

[Prasanna S Panchamukhi]

On Tue, Apr 25, 2006 at 10:21:43AM -0400, Frank Ch. Eigler wrote:
>
> Prasanna S Panchamukhi <prasanna@in.ibm.com> writes:
>
> > Please find the patch for robust fault handling of kprobes for i386
> > architecture. [...]
>
> This patch puts the setjmp into the kprobes layer, around the
> invocation of the probe handler.  A longjmp due to a systemtap probe
> problem would skip all the cleanup code (locking, error tracking,
> context cleanup) that the probe handler boilerplate includes.  I had
> previously imagined letting a probe handler include the setjmp
> explicitly, so it can have its own cleanup code in systemtap-emitted
> code.

IIUC you are planning on setjmp/longjmp() in systemtap generated
handlers themselves ?

This can be achieved even without the robust fault handling code.

>
> This scheme would mean that the subject processor will not be able to
> run any further systemtap probes, and probes on other processors will
> timeout acquiring locks on any shared variables.  This would cause the
> systemtap session to come to a prompt close due to MAXSKIPPED.
>
> There are problems associated with overall session shutdown.  The
> module removal code blocks until all running probe handler finish.
> But if it has no way of knowing that one was "stuck", it could wait a
> mighty long time.  This would likely have to be solved for the
> proposed scheme to be acceptable.
>

All that is required for systemtap pre/post handler is to allocate
a systemtap jump buffer and do a setjump() before accessing user
address space.

systemtap_jmp_buffer[]; <percpu data>

systemtap_prehandler(.....)
{
	.........
	if ((setjmp(systemtap_jmp_buffer)) != 0) {
		/* came here because of longjmp() */
		<cleanup code>
	}
	copy_from_user_inatomic(.....);
}

If such a access to user address space generates a page_fault, then
the kprobes_fault_handler() will execute the registered
systemtap_faulthandler(). The systemtap_faulthandler() is required to
change the instruction pointer to a routine that does the longjmp()
(systemtap_fault_trampoline() for example as shown below) and return
one from the systemtap_faulthandler(). The kprobes_fault_handler() just
returns without doing anything.

systemtap_faulthandler(....)
{
	regs->eip = (unsigned long) systemtap_fault_trampoline;
	return 1; /*kprobe handler need not do anything else */
}

Since the instruction pointer is changed, the system page_fault handler
will return to the systemtap longjmp() code(i.e.
systemtap_fault_trampoline()). The systemtap longjmp() will jump back
to the systemtap pre/post handler, since now systemtap knows
that fault has happened during the pre/post handler, it can do the
recovery or cleanup. After the recovery or cleanup is done, the
systemtap_prehandler() just returns.

systemtap_fault_trampoline(void)
{
	/* goto systemtap-prehandler() */
	longjmp(systemtap_jmp_buffer);
	return;
}

The robust fault handling patch will be helpful if the kprobe
user pre/post handlers or systemtap handlers does not do the
setjmp/longjmp() explicitly (non-systemtap kprobe users).

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

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

[Frank Ch. Eigler]

prasanna wrote:

> [...]  IIUC you are planning on setjmp/longjmp() in systemtap
> generated handlers themselves ?

That was one idea, yeah.

> This can be achieved even without the robust fault handling code.

Except I can't find any general setjmp/longjmp implementation in the
kernel.

> [...]
> 	if ((setjmp(systemtap_jmp_buffer)) != 0) {
> 		<cleanup code>
> 	}
> 	copy_from_user_inatomic(.....);

(The systemtap-emitted setjmp would likely be in the probe prologue
boilerplate, not associated with copy_from_user so closely.)

> [...]
> systemtap_faulthandler(....)
> 	regs->eip = (unsigned long) systemtap_fault_trampoline;
> 	return 1; /*kprobe handler need not do anything else */

"instruction_register(regs)" instead "regs->eip" for portability,
right?

> [...]
> systemtap_fault_trampoline(void)
> 	longjmp(systemtap_jmp_buffer);
> 	return;

That sounds simple enough (assuming the availability of longjmp).


- FChE