[PATCH] Kprobes: Incorrect handling of probes on ret/lret instruction

Hi,

Kprobes could not handle the insertion of a probe on the ret/lret instruction
and used to oops after single stepping since kprobes was modifying eip/rip 
incorrectly. Adjustment of eip/rip is not required after single stepping in 
case of ret/lret instruction, because eip/rip points to the correct location 
after execution of the ret/lret instruction. This patch fixes the above problem.

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

---

---

 linux-2.6.12-rc1-prasanna/arch/i386/kernel/kprobes.c   |    7 +++++++
 linux-2.6.12-rc1-prasanna/arch/x86_64/kernel/kprobes.c |    7 +++++++
 2 files changed, 14 insertions(+)

diff -puN arch/i386/kernel/kprobes.c~kprobes-ret-address-fix arch/i386/kernel/kprobes.c
--- linux-2.6.12-rc1/arch/i386/kernel/kprobes.c~kprobes-ret-address-fix	2005-03-31
14:32:56.000000000 +0530
+++ linux-2.6.12-rc1-prasanna/arch/i386/kernel/kprobes.c	2005-03-31 14:37:24.000000000 +0530
 <at>  <at>  -218,6 +218,13  <at>  <at>  static void resume_execution(struct kpro
 		*tos &= ~(TF_MASK | IF_MASK);
 		*tos |= kprobe_old_eflags;
 		break;
+	case 0xc3:		/* ret/lret */
+	case 0xcb:
+	case 0xc2:
+	case 0xca:

[RFC] Design + prototype: Multiple handler sets per probe address

Hi,

Here is a design to support "Mulitple handler sets per address". I have 
also put in the i386 implementation based on this design.

Some notes:

- The interfaces to register, unregister, define handlers all remain
the same.
- A kprobe and jprobe cannot co-exist at the same location. (Ideas are 
welcome on how to support this).

I have minimally tested the patch and it works(tm).

Please let me know your thoughts on the design. I'd also appreciate if 
you could test the patch (diffed against 2.6.12-rc1-mm3) and provide 
feedback.

Thanks,
Ananth

Draft of 1 April 2005.

1	Multiple handler sets per kprobe

1.1	Overview

One of the requirements of SystemTAP is the capability of defining 

Re: [RFC] Design + prototype: Multiple handler sets per probe address

Hi -

On Fri, Apr 01, 2005 at 09:34:28AM -0500, Ananth N Mavinakayanahalli wrote:
> Here is a design to support "Mulitple handler sets per address". I have 
> also put in the i386 implementation based on this design.

Thank you.

> 2.4.2.3 Fault handlers
> 
> Fault handlers are called in sequence _until_ one of them handles
> the fault (returns non zero). Once a fault is handled, no other
> handlers are called.

This sounds wrong.  If at all possible, the code should arrange to
call only the single fault handler that corresponds to the current
specific kprobe being handled.

Keep in mind that one use for this feature is for running several
independent systemtap sessions at the same time.  They should remain
as isolated from and ignorant of each other as possible.  In
particular, one should not be told that the other has happened to
cause a fault.  Similarly, one client should not be able to prevent
another client from getting its probe handler called, by returning a
"success" indication of its own.

> [...]
> - Multiple handlers cannot be registered at a location that already
>   has a jprobe registered. In other words, to register a jprobe,
>   no other handlers must be registered at that address and 

Re: safety paper - please send feedback!

You can safely and simply deal with procedure calls, loops and recursion by
implementing a counter which is decremented for any attempted
non-sequential execution. We did this in dprobes and called it jmpmax.  You
could allow the user to override the default counter value, but never allow
it to become disabled. If the counter is decremented to zero then the
probes is terminated.

On this basis I would allow procedure calls.
- -
Richard J Moore
IBM Advanced Linux Response Team - Linux Technology Centre
MOBEX: 264807; Mobile (+44) (0)7739-875237
Office: (+44) (0)1962-817072

                                                                           
             "Chen, Brad"                                                  
             <brad.chen <at> inte                                               
             l.com>                                                     To 
             Sent by:                   <systemtap <at> sources.redhat.com>     
             systemtap-owner                                            cc 
              <at> sources.redhat                                               
             .com                                                      bcc 

                                                                   Subject 
             23/03/2005                 safety paper - please send         
             19:14                      feedback!                          

I've collected some thoughts on Safety in Systemtap for
consideration by the group. It includes a list of possible

Re: safety paper redux

My previous comment was made against the fisrt draft, so ignore it unless
you feel it still applies.
- -
Richard J Moore
IBM Advanced Linux Response Team - Linux Technology Centre
MOBEX: 264807; Mobile (+44) (0)7739-875237
Office: (+44) (0)1962-817072

Re: [RFC] Design + prototype: Multiple handler sets per probe address

Frank Ch. Eigler wrote:
> Hi -
> 
> 
> On Fri, Apr 01, 2005 at 09:34:28AM -0500, Ananth N Mavinakayanahalli wrote:
> 
>>Here is a design to support "Mulitple handler sets per address". I have 
>>also put in the i386 implementation based on this design.
> 
> 
> Thank you.
> 
> 
> 
>>2.4.2.3 Fault handlers
>>
>>Fault handlers are called in sequence _until_ one of them handles
>>the fault (returns non zero). Once a fault is handled, no other
>>handlers are called.
> 
> 
> This sounds wrong.  If at all possible, the code should arrange to
> call only the single fault handler that corresponds to the current
> specific kprobe being handled.
> 
> Keep in mind that one use for this feature is for running several
> independent systemtap sessions at the same time.  They should remain
> as isolated from and ignorant of each other as possible.  In
> particular, one should not be told that the other has happened to
> cause a fault.  Similarly, one client should not be able to prevent

what is a tapset? [RE: Skeleton detailed design document]

Looking this over the draft paper, and thinking about Frank's 
note concerning script libraries, I started thinking about 
the first figure suggested in the draft. See my ASCII art 
below. Perhaps this is overly simplistic; is any of this 
incorrect? What parts are missing?

===========================================================

INPUT:  script
	     |
	     v
TOOLS: elaborator -> compiler ->   ld   -> mytap.ko
           ^             ^          ^
           |             |          |
	tapset defns       |       systemtap libs
	   		       |       tapset libs
			       |       kernel libs
				 |
		   /usr/include/systemtap/
		   /usr/include/systemtap/tapset/
               kernel symbolic information

EXECUTION:	insmod("mytap.ko") -> [kernel]
					[kjr]probes + trap handler
      	      	      systemtap runtime in kernel [needed?]
					user-level data handler 

===========================================================

Re: what is a tapset? [RE: Skeleton detailed design document]

Hi -

On Fri, Apr 01, 2005 at 12:18:47PM -0800, Chen, Brad wrote:
> [...]. Perhaps this is overly simplistic; is any of this 
> incorrect? What parts are missing?

Looks okay, except that I am not sure that the "tapset" constructs
I've seen so far need *any* C code (custom headers or libraries).  I
would like to see whether "tapsets" can be restricted entirely to live
within script fragments that the elaboration phase would pull in.

By the way, the kernel symbolic information would be under
/usr/lib/debug or /usr/src/debug, whereever kernel-debuginfo RPMs
deposit their goods.

> [...]
> Question: Can two taps "mytap1.ko" and "mytap2.ko" be loaded
> in the kernel at the same time? 

Concurrent independent systemtap runs should not be precluded.

> If so, do they need to share anything besides kprobes? 

So far, no.  However, when it comes to sharing other resources such as
the perfctr bits, some arbitration code will have to be written into
the runtime.

> [...]
> - Do the tapset libraries need to be built with the kernel?
>   [yes; they will reference kernel symbols that must be

Re: [RFC] Design + prototype: Multiple handler sets per probe address

Hi -

On Fri, Apr 01, 2005 at 11:15:58AM -0500, Ananth N Mavinakayanahalli wrote:
> >[...]
> >Keep in mind that one use for this feature is for running several
> >independent systemtap sessions at the same time.  [...]
> 
> Good point. One way of tracking which handler caused the fault
> would be to have a struct kprobe *current_handler. [...]

Yes, except that global variables are a pain from the point of view of
proper support for concurrent kprobe execution on an SMP.  Maybe it's
as simple as keeping per-cpu "global" state only.

> >Could you be more specific about why this limitation needs to exist?
> >[...]
> The design of jprobes is such [...]

Thanks for the explanation.  I don't quite grok it yet, but that's
okay, but it sounds complex enough to force us to accept an
incompatibility between jprobes and kprobes.

> >[...]  It would be nice if some counter was kept of the number of
> >times that a probe was skipped due to reasons like this, so that
> >ultimately a user become aware that something went wrong.
>
> [...]
> I don't get the a counter idea though. When we recognize recursion
> the first time, the trap instruction at the offending address is
> removed and so we don't register subsequent hits at all.

Re: [RFC] Design + prototype: Multiple handler sets per probe address

Ananth N Mavinakayanahalli wrote:
> Hi,
> 
> Here is a design to support "Mulitple handler sets per address". I have 
> also put in the i386 implementation based on this design.
> 
> Some notes:
> 
> - The interfaces to register, unregister, define handlers all remain
> the same.
> - A kprobe and jprobe cannot co-exist at the same location. (Ideas are 
> welcome on how to support this).
> 
> I have minimally tested the patch and it works(tm).
> 
> Please let me know your thoughts on the design. I'd also appreciate if 
> you could test the patch (diffed against 2.6.12-rc1-mm3) and provide 
> feedback.
> 
> 
> Thanks,
> Ananth

I have been working on some simple testing to exercises the multiple 
probes. It checks to make sure that probes are inserted and deleted as 
expected. It has three probes at location.

I appologize for the verbose output. I have also attached output of the 
test results. There are some warning about allocating sleeping function.