Message-ID: <54F01D71.1030303@hitachi.com>
Date: Fri, 27 Feb 2015 16:32:01 +0900
From: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Organization: Hitachi, Ltd., Japan
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1
MIME-Version: 1.0
To: Petr Mladek <pmladek@suse.cz>
Cc: "David S. Miller" <davem@davemloft.net>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>,
        Ananth N Mavinakayanahalli <ananth@in.ibm.com>,
        Frederic Weisbecker <fweisbec@gmail.com>,
        Ingo Molnar <mingo@redhat.com>, Steven Rostedt <rostedt@goodmis.org>,
        Jiri Kosina <jkosina@suse.cz>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/7] kprobe: Handle error when Kprobe ftrace arming fails
References: <1424967232-2923-1-git-send-email-pmladek@suse.cz>
In-Reply-To: <1424967232-2923-1-git-send-email-pmladek@suse.cz>
Content-Type: text/plain; charset=ISO-2022-JP
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2609
Lines: 68

Hi Petr,

(2015/02/27 1:13), Petr Mladek wrote:
> arm_kprobe_ftrace() could fail, especially after introducing ftrace IPMODIFY
> flag and LifePatching. This patch set adds the error handling and also some
> related fixes.

Hmm, I'd like to drop IPMODIFY from kprobes except for jprobes,
since it actually doesn't change regs->ip which was sent before.
It seems that this series partly covers that work.

> 1st patch includes the most important change. It helps to keep Kprobes
> in a sane state.
> 
> 2nd and 3rd patch allows to propagate the error where needed.

OK, I think the 1st one could be merged. 2nd and 3rd one still have some
issues as far as I reviewed.

> The other patches fix problems with the global kprobes_all_disarmed flag.
> They were there even before but they become more visible and critical
> after the arming errors became propagated.

Could you separate the series? And also I doubt we need to show global
disable status, since we can check it via debugfs too (and looks redundant).

Thank you,

> The first patch looks rather safe and might be suitable even for 4.0.
> 
> However, I would feel more comfortable if the other patches get some
> testing in linux-next. I did quite some testing and did my best. But
> I started with the three patches and was surprised by the effect of
> the propagated errors. They triggered that BUG_ON() in
> __unregister_kprobe_top() are required the other patches
> to get it working. I wonder if there is any other scenario that
> I have missed.
> 
> Of course, I also wait for feedback how to make things better.
> 
> 
> Petr Mladek (7):
>   kprobes: Disable Kprobe when ftrace arming fails
>   kprobes: Propagate error from arm_kprobe_ftrace()
>   kprobes: Propagate error from disarm_kprobe_ftrace()
>   kprobes: Keep consistent state of kprobes_all_disarmed
>   kprobes: Do not try to disarm already disarmed Kprobe
>   kprobes: Check kprobes_all_disarmed in kprobe_disarmed()
>   kprobes: Mark globally disabled Kprobes in debugfs interface
> 
>  Documentation/kprobes.txt |   5 +-
>  kernel/kprobes.c          | 279 ++++++++++++++++++++++++++++++++++------------
>  2 files changed, 213 insertions(+), 71 deletions(-)
> 


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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/