Re: Re: [PATCH 6/9][RFC] kprobes: Allow probe on ftrace reserved text (but move it)

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2012/05/03 8:40), Steven Rostedt wrote:
> On Wed, 2012-05-02 at 16:40 -0400, Frank Ch. Eigler wrote:
>> rostedt wrote:
>>
>>> [...]  Added KPROBE_FLAG_MOVED (as suggested by Masami) that is set
>>> when the address is moved to get around an ftrace nop. [...]
>>
>> Steve, perhaps my earlier comments on this got lost during the mailing
>> list outage.
> 
> I saw it, but it didn't really specify what you wanted. Here's your
> comment:
> 
> 
>> I suspect Masami intended that this flag is later used during int3
>> processing to subtract MCOUNT_INSN_SIZE back out from the pt_regs->ip
>> during kprobe_handler() if this flag was set.
> 
> This is what I thought too, but to me it sounded like Masami could do
> the work. I was just setting up a flag to make it possible.
> 
>>
>> The gist is that a KPROBE_FLAG_MOVED being set this way accomplishes
>> very little since nothing is looking for that flag.  Instead, you
>> should patch {arch/*}/kernel/kprobe.c kprobe_handler() to subtract
>> MCOUNT_INSN_SIZE back out from pt_regs->ip if KPROBE_FLAG_MOVED was
>> set.  That way, kprobes clients need do not perceive the int3 movement.
> 
> I basically thought that Masami wanted me to add the flag, and then
> others could look for this and do the adjustment. I'm not the kprobes
> author. I was just adding a flag that Masami and others could use to do
> such updates.

Right, that was what I thought. Since the kp->addr is changed when
kprobe is set, kprobes itself don't need to adjust the pt_regs->ip.
I mean, struct kprobe itself puts a probe on the next to the mcount
entry, even if the caller tries to put a probe on the mcount entry.

This change may be unintended and caller will doubt that why the
kp->addr is automatically changed. So this KPROBE_FLAG_MOVED gives
a hint for the caller who knows the original intended probed address.

> I'm not sure if the adjustment is fine with everyone, as it may cause
> repercussions that I don't know about. 

Yeah, that's a point. if the adjustment is transparently done, there
is no problem. But it changes kp->addr when registering a probe.
If adjustment is done, following code (still) doesn't work.

---
int func(struct kprobe *kp, strcut pt_regs *regs)
{
	BUG_ON(kp->addr != regs->ip);
	/* or */
	store_probed_address(kp->addr);	/* since regs->ip depends on x86*/
}

kp->handler = func;
kp->addr = <somewhere on ftrace>
register_kprobe(kp);
---

but if adjustment is not done, at least, kprobes behavior itself
looks same. (but just be moved if probed on ftrace)

Yeah, I know systemtap people likes regs->ip to be adjusted, but
there may be someone who use raw kprobes.

> Perhaps that could be another patch (want to write it?)

Oh, so I think we need to show the new flag on debugfs for
someone who want to know why the probe has been moved. :)


By the way, there is another way to do that transparently which
we add a "real_addr" member to struct kprobes and put the real
probed address to the member (without changing kp->addr). This
will keep API compatibility.

Thank you,


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com