From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12665 invoked by alias); 23 Feb 2006 08:58:06 -0000 Received: (qmail 12657 invoked by uid 22791); 23 Feb 2006 08:58:05 -0000 X-Spam-Status: No, hits=-0.7 required=5.0 tests=AWL,BAYES_00,DNS_FROM_RFC_ABUSE,SPF_SOFTFAIL X-Spam-Check-By: sourceware.org Received: from e3.ny.us.ibm.com (HELO e3.ny.us.ibm.com) (32.97.182.143) by sourceware.org (qpsmtpd/0.31) with ESMTP; Thu, 23 Feb 2006 08:58:03 +0000 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e3.ny.us.ibm.com (8.12.11/8.12.11) with ESMTP id k1N8vxkM000813 for ; Thu, 23 Feb 2006 03:57:59 -0500 Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.12.10/NCO/VERS6.8) with ESMTP id k1N8vxLd232318 for ; Thu, 23 Feb 2006 03:57:59 -0500 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.12.11/8.13.3) with ESMTP id k1N8vxp0014892 for ; Thu, 23 Feb 2006 03:57:59 -0500 Received: from newton.in.ibm.com ([9.124.35.47]) by d01av02.pok.ibm.com (8.12.11/8.12.11) with ESMTP id k1N8vv0R014850; Thu, 23 Feb 2006 03:57:58 -0500 Received: by newton.in.ibm.com (Postfix, from userid 500) id 84677CE3; Thu, 23 Feb 2006 14:29:35 +0530 (IST) Date: Thu, 23 Feb 2006 08:58:00 -0000 From: Prasanna S Panchamukhi To: "Mao, Bibo" Cc: systemtap@sources.redhat.com Subject: Re: [PATCH] Kprobes- robust fault handling for i386 Message-ID: <20060223085935.GA2473@in.ibm.com> Reply-To: prasanna@in.ibm.com References: <9FBCE015AF479F46B3B410499F3AE05BC4BF0C@pdsmsx405> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9FBCE015AF479F46B3B410499F3AE05BC4BF0C@pdsmsx405> User-Agent: Mutt/1.4.1i X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2006-q1/txt/msg00590.txt.bz2 Bibo, You are right, the esp might be local variable, we are working on it. As of now to fix the broken kprobes fault handling, here is the patch. This is only for i386, once we freeze on this prototype, this can be ported to other architectures. Thanks Prasanna This patch provides proper kprobes fault handling, if a user-specified pre/post 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 arch/i386/kernel/kprobes.c | 78 ++++++++++++++++++++++++++++++++++++++------- include/linux/kprobes.h | 2 + 2 files changed, 69 insertions(+), 11 deletions(-) diff -puN arch/i386/kernel/kprobes.c~kprobes-i386-pagefault-handling arch/i386/kernel/kprobes.c --- linux-2.6.16-rc3-mm1/arch/i386/kernel/kprobes.c~kprobes-i386-pagefault-handling 2006-02-23 11:41:29.000000000 +0530 +++ linux-2.6.16-rc3-mm1-prasanna/arch/i386/kernel/kprobes.c 2006-02-23 14:09:15.000000000 +0530 @@ -35,6 +35,7 @@ #include #include #include +#include void jprobe_return_end(void); @@ -220,8 +221,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; @@ -308,7 +310,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: @@ -509,7 +514,9 @@ static inline int post_kprobe_handler(st if (!cur) return 0; - if ((kcb->kprobe_status != KPROBE_REENTER) && cur->post_handler) { + if ((kcb->kprobe_status != KPROBE_REENTER) + && (kcb->kprobe_status != KPROBE_HIT_FAULT_SS) + && cur->post_handler) { kcb->kprobe_status = KPROBE_HIT_SSDONE; cur->post_handler(cur, regs, 0); } @@ -542,15 +549,64 @@ 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; + /* fall down and fixup the exception*/ + case KPROBE_HIT_SSDONE: + /* + * 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-rc3-mm1/include/linux/kprobes.h~kprobes-i386-pagefault-handling 2006-02-23 12:08:07.000000000 +0530 +++ linux-2.6.16-rc3-mm1-prasanna/include/linux/kprobes.h 2006-02-23 12:31:21.000000000 +0530 @@ -46,6 +46,8 @@ #define KPROBE_HIT_SS 0x00000002 #define KPROBE_REENTER 0x00000004 #define KPROBE_HIT_SSDONE 0x00000008 +#define KPROBE_HIT_FAULT 0x00000010 +#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