(2015/02/27 1:13), Petr Mladek wrote:
> arm_kprobe_ftrace() could fail, especially after introducing ftrace IPMODIFY
> flag and LifePatching. But this situation is not properly handled.
> This patch adds the most important changes.
Hmm, as you know, I actually working on it to drop IPMODIFY from kprobes except
jprobe.
however, yes, that is not enough. We might better set DISABLED flag as
this does.
> First, it does not make sense to register "kprobe_ftrace_ops" if the filter
> was
> not set.
>
> Second, we should remove the filter if the registration of "kprobe_ftrace_ops"
> fails. The failure might be caused by conflict between the Kprobe and
> a life patch via the IPMODIFY flag. If we remove the filter, we will allow
> to register "kprobe_ftrace_ops" for another non-conflicting Kprobe later.
>
> Third, we need to make sure that "kprobe_ftrace_enabled" is incremented only
> when "kprobe_ftrace_ops" is successfully registered. Otherwise, another
> Kprobe will not try to register it again. Note that we could move the
> manipulation with this counter because it is accessed only under
> "kprobe_mutex".
>
> Four, we should mark the probe as disabled if the ftrace stuff is not usable.
> It will be the correct status. Also it will prevent the unregistration code
> from producing another failure.
>
> It looks more safe to disable the Kprobe directly in "kprobe_ftrace_ops". Note
> that we need to disable also all listed Kprobes in case of an aggregated
> probe.
> It would be enough to disable only the new one but we do not know which one it
> was. They should be in sync anyway.
>
Acked-by: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
Thank you!
> Signed-off-by: Petr Mladek <pmla...@suse.cz>
> ---
> kernel/kprobes.c | 25 +++++++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index ee619929cf90..d1b9db690b9c 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -931,16 +931,33 @@ static int prepare_kprobe(struct kprobe *p)
> /* Caller must lock kprobe_mutex */
> static void arm_kprobe_ftrace(struct kprobe *p)
> {
> + struct kprobe *kp;
> int ret;
>
> ret = ftrace_set_filter_ip(&kprobe_ftrace_ops,
> (unsigned long)p->addr, 0, 0);
> - WARN(ret < 0, "Failed to arm kprobe-ftrace at %p (%d)\n", p->addr, ret);
> - kprobe_ftrace_enabled++;
> - if (kprobe_ftrace_enabled == 1) {
> + if (WARN(ret < 0,
> + "Failed to arm kprobe-ftrace at %p (%d). The kprobe gets
> disabled.\n",
> + p->addr, ret))
> + goto err_filter;
> +
> + if (!kprobe_ftrace_enabled) {
> ret = register_ftrace_function(&kprobe_ftrace_ops);
> - WARN(ret < 0, "Failed to init kprobe-ftrace (%d)\n", ret);
> + if (WARN(ret < 0,
> + "Failed to init kprobe-ftrace (%d). The probe at %p
> gets disabled\n",
> + ret, p->addr))
> + goto err_function;
> }
> + kprobe_ftrace_enabled++;
> + return;
> +
> +err_function:
> + ftrace_set_filter_ip(&kprobe_ftrace_ops, (unsigned long)p->addr, 1, 0);
> +err_filter:
> + p->flags |= KPROBE_FLAG_DISABLED;
> + if (kprobe_aggrprobe(p))
> + list_for_each_entry_rcu(kp, &p->list, list)
> + kp->flags |= KPROBE_FLAG_DISABLED;
> }
>
> /* Caller must lock kprobe_mutex */
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
