All of lore.kernel.org
 help / color / mirror / Atom feed
From: tip-bot for Petr Mladek <tipbot@zytor.com>
To: linux-tip-commits@vger.kernel.org
Cc: rostedt@goodmis.org, linux-kernel@vger.kernel.org, hpa@zytor.com,
	mingo@kernel.org, anil.s.keshavamurthy@intel.com,
	masami.hiramatsu.pt@hitachi.com, fweisbec@gmail.com,
	pmladek@suse.cz, jkosina@suse.cz, ananth@in.ibm.com,
	tglx@linutronix.de, davem@davemloft.net
Subject: [tip:perf/urgent] kprobes/x86: Check for invalid ftrace location in __recover_probed_insn()
Date: Sat, 21 Feb 2015 09:46:43 -0800	[thread overview]
Message-ID: <tip-2a6730c8b6e075adf826a89a3e2caa705807afdb@git.kernel.org> (raw)
In-Reply-To: <1424441250-27146-3-git-send-email-pmladek@suse.cz>

Commit-ID:  2a6730c8b6e075adf826a89a3e2caa705807afdb
Gitweb:     http://git.kernel.org/tip/2a6730c8b6e075adf826a89a3e2caa705807afdb
Author:     Petr Mladek <pmladek@suse.cz>
AuthorDate: Fri, 20 Feb 2015 15:07:30 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Sat, 21 Feb 2015 10:33:31 +0100

kprobes/x86: Check for invalid ftrace location in __recover_probed_insn()

__recover_probed_insn() should always be called from an address
where an instructions starts. The check for ftrace_location()
might help to discover a potential inconsistency.

This patch adds WARN_ON() when the inconsistency is detected.
Also it adds handling of the situation when the original code
can not get recovered.

Suggested-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Petr Mladek <pmladek@suse.cz>
Cc: Ananth NMavinakayanahalli <ananth@in.ibm.com>
Cc: Anil S Keshavamurthy <anil.s.keshavamurthy@intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Steven Rostedt <rostedt@goodmis.org>
Link: http://lkml.kernel.org/r/1424441250-27146-3-git-send-email-pmladek@suse.cz
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/kprobes/core.c | 12 ++++++++++++
 arch/x86/kernel/kprobes/opt.c  |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index c3b4b46..4e3d5a9 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -228,6 +228,13 @@ __recover_probed_insn(kprobe_opcode_t *buf, unsigned long addr)
 	kp = get_kprobe((void *)addr);
 	faddr = ftrace_location(addr);
 	/*
+	 * Addresses inside the ftrace location are refused by
+	 * arch_check_ftrace_location(). Something went terribly wrong
+	 * if such an address is checked here.
+	 */
+	if (WARN_ON(faddr && faddr != addr))
+		return 0UL;
+	/*
 	 * Use the current code if it is not modified by Kprobe
 	 * and it cannot be modified by ftrace.
 	 */
@@ -265,6 +272,7 @@ __recover_probed_insn(kprobe_opcode_t *buf, unsigned long addr)
  * Recover the probed instruction at addr for further analysis.
  * Caller must lock kprobes by kprobe_mutex, or disable preemption
  * for preventing to release referencing kprobes.
+ * Returns zero if the instruction can not get recovered.
  */
 unsigned long recover_probed_instruction(kprobe_opcode_t *buf, unsigned long addr)
 {
@@ -299,6 +307,8 @@ static int can_probe(unsigned long paddr)
 		 * normally used, we just go through if there is no kprobe.
 		 */
 		__addr = recover_probed_instruction(buf, addr);
+		if (!__addr)
+			return 0;
 		kernel_insn_init(&insn, (void *)__addr, MAX_INSN_SIZE);
 		insn_get_length(&insn);
 
@@ -347,6 +357,8 @@ int __copy_instruction(u8 *dest, u8 *src)
 	unsigned long recovered_insn =
 		recover_probed_instruction(buf, (unsigned long)src);
 
+	if (!recovered_insn)
+		return 0;
 	kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
 	insn_get_length(&insn);
 	/* Another subsystem puts a breakpoint, failed to recover */
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 7c523bb..3aef248 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -259,6 +259,8 @@ static int can_optimize(unsigned long paddr)
 			 */
 			return 0;
 		recovered_insn = recover_probed_instruction(buf, addr);
+		if (!recovered_insn)
+			return 0;
 		kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
 		insn_get_length(&insn);
 		/* Another subsystem puts a breakpoint */

      reply	other threads:[~2015-02-21 17:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-20 14:07 [PATCH v3 0/2] kprobes/x86: Fix up interaction between kprobes code recovery and ftrace Petr Mladek
2015-02-20 14:07 ` [PATCH v3 1/2] kprobes/x86: Use 5-byte NOP when the code might be modified by ftrace Petr Mladek
2015-02-21 17:46   ` [tip:perf/urgent] " tip-bot for Petr Mladek
2015-02-20 14:07 ` [PATCH v3 2/2] kprobes/x86: Check for invalid ftrace location in __recover_probed_insn() Petr Mladek
2015-02-21 17:46   ` tip-bot for Petr Mladek [this message]

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=tip-2a6730c8b6e075adf826a89a3e2caa705807afdb@git.kernel.org \
    --to=tipbot@zytor.com \
    --cc=ananth@in.ibm.com \
    --cc=anil.s.keshavamurthy@intel.com \
    --cc=davem@davemloft.net \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=jkosina@suse.cz \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tip-commits@vger.kernel.org \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=pmladek@suse.cz \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.