mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.cz>
To: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	"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>
Cc: Ingo Molnar <mingo@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Jiri Kosina <jkosina@suse.cz>,
	linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.cz>
Subject: [PATCH 1/7] kprobes: Disable Kprobe when ftrace arming fails
Date: Thu, 26 Feb 2015 17:13:46 +0100	[thread overview]
Message-ID: <1424967232-2923-2-git-send-email-pmladek@suse.cz> (raw)
In-Reply-To: <1424967232-2923-1-git-send-email-pmladek@suse.cz>

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.

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.

Signed-off-by: Petr Mladek <pmladek@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 */
-- 
1.8.5.6


  reply	other threads:[~2015-02-26 16:13 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-26 16:13 [PATCH 0/7] kprobe: Handle error when Kprobe " Petr Mladek
2015-02-26 16:13 ` Petr Mladek [this message]
2015-02-27  6:26   ` [PATCH 1/7] kprobes: Disable Kprobe when " Masami Hiramatsu
2015-02-26 16:13 ` [PATCH 2/7] kprobes: Propagate error from arm_kprobe_ftrace() Petr Mladek
2015-02-27  7:35   ` Masami Hiramatsu
2015-02-26 16:13 ` [PATCH 3/7] kprobes: Propagate error from disarm_kprobe_ftrace() Petr Mladek
2015-02-27  8:01   ` Masami Hiramatsu
2015-02-26 16:13 ` [PATCH 4/7] kprobes: Keep consistent state of kprobes_all_disarmed Petr Mladek
2015-02-26 16:13 ` [PATCH 5/7] kprobes: Do not try to disarm already disarmed Kprobe Petr Mladek
2015-02-26 16:13 ` [PATCH 6/7] kprobes: Check kprobes_all_disarmed in kprobe_disarmed() Petr Mladek
2015-02-26 16:13 ` [PATCH 7/7] kprobes: Mark globally disabled Kprobes in debugfs interface Petr Mladek
2015-02-27  7:32 ` [PATCH 0/7] kprobe: Handle error when Kprobe ftrace arming fails Masami Hiramatsu
2015-03-12 16:33   ` Petr Mladek
2015-03-13 12:36     ` Masami Hiramatsu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1424967232-2923-2-git-send-email-pmladek@suse.cz \
    --to=pmladek@suse.cz \
    --cc=ananth@in.ibm.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=davem@davemloft.net \
    --cc=fweisbec@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®