Re: [RFC][PATCH] PPC64: return address probes

[Jim Keniston <jkenisto at us dot ibm dot com>]

On Mon, 2005-05-23 at 05:03, Ananth N Mavinakayanahalli wrote:
> Jim Keniston wrote:
> > On Fri, 2005-05-20 at 05:15, Ananth N Mavinakayanahalli wrote:
> 
> Hi Jim,
> 
> Thanks for the comments...
> 
> >>Hi,
> >>
> >>Here is the return address probes prototype for ppc64. This patch
> >>applies on 2.6.12-rc4-mm2 + Rusty's arch_break_inst.patch and Hien's
> >>kprobe_flush.patch.
> > 
> > ...
...
> > 
> > This looks wrong to me.  It seems to me that &regs->link is in the
> > middle of the pt_regs struct, which will disappear as soon as we return
> > from the trap.  To be of use to arch_kprobe_flush_task(),
> > ri->arch_retaddr.linkreg_ptr should point to the place in the
> > yet-to-be-constructed stack frame of the probed function where the link
> > register will be saved.
> 
> Yes, this is incorrect - I'll fix that (come to think of it, I am not
> sure how I could assume that :-). However, I don't see an easy way to
> determine where the location for restoring the value. Also, you may
> never take a stack frame unless there is another function call from
> within the return address probed function.

Correct.  I don't see an easy solution, either.

> 
> > If this location is always predictable, and does indeed turn out to be
> > equal to &regs->link, then you should add a comment explaining the
> > coincidence.  I was under the impression that it's not predictable from
> > the information available to kprobes.  If that's the case, then I guess
> > that arch_kprobe_flush_task() can't do the restore-the-return-address
> > trick.  This would affect only return-probed functions that are active
> > when arch_kprobe_flush_task() is called, and actually return.  I think
> > that this is a fairly small class of functions; flush_old_exec() would
> > be one.
> 
> As I see in the code, do_exit() calls exit_thread() but doesn't "return" 
> and as you mention, I see that flush_old_exec() is the only function
> calls flush_tread() and does a return.
> 
> What do you suggest would be a good way to handle this? Do we put a
> caveat that users can't have return probes on flush_old_exec()? Any
> other ideas?

I believe that a caveat is sufficient.  If we wanted to be more
bullet-proof, register_kretprobe() could refuse to establish a return
probe on flush_thread() or flush_old_exec().

> 
> Ananth
> 

Jim