Re: [PATCH ftrace/core v6 4/5] kprobes: Set IPMODIFY flag only if the probe can change regs->ip

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

Hi Petr,

Sorry I missed this mail.

(2015/01/27 1:14), Petr Mladek wrote:> On Fri 2014-11-21 05:25:30, Masami Hiramatsu wrote:
>> Set FTRACE_OPS_FL_IPMODIFY flag only for the probes which can change
>> regs->ip, which has kprobe->break_handler.
>> Currently we can not put jprobe and another ftrace handler which
>> changes regs->ip on the same function because all kprobes have
>> FTRACE_OPS_FL_IPMODIFY flag. This removes FTRACE_OPS_FL_IPMODIFY
>> flag from kprobes and only when the user uses jprobe (or the
>> kprobe.break_handler != NULL) we add additinal ftrace_ops with
>> FTRACE_OPS_FL_IPMODIFY on target function.
>
> Please, what are the plans with this patch?

Well, I'll revise this for newer kernel.

>
> I have checked the interference between Kprobes and LivePatching and
> here is my observation:
>
> 1. Jprobe and LivePatch must be in a hard conflict. They both need
>    to change IP and continue another way after ftrace ops finishes.
>
>    BTW: I wonder a bit why Jprobe handler could not be called directly
>    from kprobe_ftrace_handler(). I guess that it is because we want
>    to call the kprobe handler in a sane context: preemption and IRQs
>    enabled, be able to use traced functions.

Right, Jprobe is just a different interface of kprobe handler. It must be
called from kprobes.
However, I think this is not so hard in practice, since we already have
perf-probe which allows us to find which register or stack is assigned to
which function parameter. That was the main reason why jprobe is introduced.
But now, we have perf-probe or systemtap, we don't(or less) need the hack like
jprobe anymore.


> 2. Normal Kprobe for the original function is ignored if the function
>    is patched.
>
>    I am working on a code that will print warning in both
>    cases. First, when we add a patch and the function has
>    a Kprobe registered. Second, the function is patched and
>    we want to add Kprobe for the original version.

Thanks! Maybe we can add "Ignored" flag for those kprobes so that users
can check it is working or not via debugfs.

>    I want to make it generic and make it dependent on the
>    IPMODIFY flag. IMHO, it just could be a handshake between
>    kprobe and ftrace code. I am still trying to understand
>    the needed parts of the code ;-)

BTW, the kprobes on function entry (iow, ftrace-based kprobes) should
not be ignored. Even if we patches a function-body, the entrance
address should be same.


> 3. Kretprobe could live with a patch without a problem!
>
>    The Kretprobe entry handler is called directly in
>    kprobe_ftrace_handler() and does not change IP.
>    On the other hand the LivePatch ftrace handler does
>    not modify the return address because the return address
>    is the same for the original and the patched function.

Right.

>
>    Or did I miss something?
>
>    This is where this patch might be useful. The other patches
>    from this patch set are already in Linus' tree and I cannot
>    find any information about this one.

Well, thank you for picking it up!


>
>    I could try to solve remaining problems if there are any.
>

Thank you,


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