linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Masami Hiramatsu <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@redhat.com>,
	linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org,
	Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>,
	Michael Ellerman <mpe@ellerman.id.au>
Subject: [RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry
Date: Tue,  7 Mar 2017 16:17:36 +0530	[thread overview]
Message-ID: <1bcc13b27c8338f7b12c160ac049029e2007cc99.1488881988.git.naveen.n.rao@linux.vnet.ibm.com> (raw)
In-Reply-To: <cover.1488881988.git.naveen.n.rao@linux.vnet.ibm.com>
In-Reply-To: <cover.1488881988.git.naveen.n.rao@linux.vnet.ibm.com>

perf specifies an offset from _text and since this offset is fed
directly into the arch-specific helper, kprobes tracer rejects
installation of kretprobes through perf. Fix this by looking up the
actual offset from a function for the specified sym+offset.

Refactor and reuse existing routines to limit code duplication -- we
repurpose kprobe_addr() for determining final kprobe address and we
split out the function entry offset determination into a separate
generic helper.

Before patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7ff]
  Probe point found: do_open+0
  Matched function: do_open [35d76dc]
  found inline addr: 0xc0000000004ba9c4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469776
  Failed to write event: Invalid argument
    Error: Failed to add events. Reason: Invalid argument (Code: -22)
  naveen@ubuntu:~/linux/tools/perf$ dmesg | tail
  <snip>
  [   33.568656] Given offset is not valid for return probe.

After patch:

  naveen@ubuntu:~/linux/tools/perf$ sudo ./perf probe -v do_open%return
  probe-definition(0): do_open%return
  symbol:do_open file:(null) line:0 offset:0 return:1 lazy:(null)
  0 arguments
  Looking at the vmlinux_path (8 entries long)
  Using /boot/vmlinux for symbols
  Open Debuginfo file: /boot/vmlinux
  Try to find probe point from debuginfo.
  Matched function: do_open [2d0c7d6]
  Probe point found: do_open+0
  Matched function: do_open [35d76b3]
  found inline addr: 0xc0000000004ba9e4
  Failed to find "do_open%return",
   because do_open is an inlined function and has no return point.
  An error occurred in debuginfo analysis (-22).
  Trying to use symbols.
  Opening /sys/kernel/debug/tracing//README write=0
  Opening /sys/kernel/debug/tracing//kprobe_events write=1
  Writing event: r:probe/do_open _text+4469808
  Writing event: r:probe/do_open_1 _text+4956344
  Added new events:
    probe:do_open        (on do_open%return)
    probe:do_open_1      (on do_open%return)

  You can now use it in all perf tools, such as:

	  perf record -e probe:do_open_1 -aR sleep 1

  naveen@ubuntu:~/linux/tools/perf$ sudo cat /sys/kernel/debug/kprobes/list
  c000000000041370  k  kretprobe_trampoline+0x0    [OPTIMIZED]
  c0000000004ba0b8  r  do_open+0x8    [DISABLED]
  c000000000443430  r  do_open+0x0    [DISABLED]

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 include/linux/kprobes.h     |  1 +
 kernel/kprobes.c            | 40 ++++++++++++++++++++++++++--------------
 kernel/trace/trace_kprobe.c |  2 +-
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index 177bdf6c6aeb..47e4da5b4fa2 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -268,6 +268,7 @@ extern void show_registers(struct pt_regs *regs);
 extern void kprobes_inc_nmissed_count(struct kprobe *p);
 extern bool arch_within_kprobe_blacklist(unsigned long addr);
 extern bool arch_function_offset_within_entry(unsigned long offset);
+extern bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset);
 
 extern bool within_kprobe_blacklist(unsigned long addr);
 
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 448759d4a263..32e6ac5131ed 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1391,21 +1391,19 @@ bool within_kprobe_blacklist(unsigned long addr)
  * This returns encoded errors if it fails to look up symbol or invalid
  * combination of parameters.
  */
-static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+static kprobe_opcode_t *_kprobe_addr(kprobe_opcode_t *addr,
+			const char *symbol_name, unsigned int offset)
 {
-	kprobe_opcode_t *addr = p->addr;
-
-	if ((p->symbol_name && p->addr) ||
-	    (!p->symbol_name && !p->addr))
+	if ((symbol_name && addr) || (!symbol_name && !addr))
 		goto invalid;
 
-	if (p->symbol_name) {
-		kprobe_lookup_name(p->symbol_name, addr);
+	if (symbol_name) {
+		kprobe_lookup_name(symbol_name, addr);
 		if (!addr)
 			return ERR_PTR(-ENOENT);
 	}
 
-	addr = (kprobe_opcode_t *)(((char *)addr) + p->offset);
+	addr = (kprobe_opcode_t *)(((char *)addr) + offset);
 	if (addr)
 		return addr;
 
@@ -1413,6 +1411,11 @@ static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
 	return ERR_PTR(-EINVAL);
 }
 
+static kprobe_opcode_t *kprobe_addr(struct kprobe *p)
+{
+	return _kprobe_addr(p->addr, p->symbol_name, p->offset);
+}
+
 /* Check passed kprobe is valid and return kprobe in kprobe_table. */
 static struct kprobe *__get_valid_kprobe(struct kprobe *p)
 {
@@ -1880,19 +1883,28 @@ bool __weak arch_function_offset_within_entry(unsigned long offset)
 	return !offset;
 }
 
+bool function_offset_within_entry(kprobe_opcode_t *addr, const char *sym, unsigned long offset)
+{
+	kprobe_opcode_t *kp_addr = _kprobe_addr(addr, sym, offset);
+
+	if (IS_ERR(kp_addr))
+		return false;
+
+	if (!kallsyms_lookup_size_offset((unsigned long)kp_addr, NULL, &offset) ||
+						!arch_function_offset_within_entry(offset))
+		return false;
+
+	return true;
+}
+
 int register_kretprobe(struct kretprobe *rp)
 {
 	int ret = 0;
 	struct kretprobe_instance *inst;
 	int i;
 	void *addr;
-	unsigned long offset;
-
-	addr = kprobe_addr(&rp->kp);
-	if (!kallsyms_lookup_size_offset((unsigned long)addr, NULL, &offset))
-		return -EINVAL;
 
-	if (!arch_function_offset_within_entry(offset))
+	if (!function_offset_within_entry(rp->kp.addr, rp->kp.symbol_name, rp->kp.offset))
 		return -EINVAL;
 
 	if (kretprobe_blacklist_size) {
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 2b7d0dd938ba..861d98fee9ea 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -696,7 +696,7 @@ static int create_trace_kprobe(int argc, char **argv)
 			return ret;
 		}
 		if (offset && is_return &&
-		    !arch_function_offset_within_entry(offset)) {
+		    !function_offset_within_entry(NULL, symbol, offset)) {
 			pr_info("Given offset is not valid for return probe.\n");
 			return -EINVAL;
 		}
-- 
2.11.1

  parent reply	other threads:[~2017-03-07 10:48 UTC|newest]

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-22 13:53 [PATCH v2 0/5] kretprobe fixes Naveen N. Rao
2017-02-22 13:53 ` [PATCH v2 1/5] kretprobes: ensure probe location is at function entry Naveen N. Rao
2017-02-22 13:53 ` [PATCH v2 2/5] powerpc: kretprobes: override default function entry offset Naveen N. Rao
2017-02-24 19:57   ` Arnaldo Carvalho de Melo
2017-02-27 12:56     ` Michael Ellerman
2017-02-25  2:45   ` Ananth N Mavinakayanahalli
2017-02-22 13:53 ` [PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses Naveen N. Rao
2017-02-27 16:32   ` Steven Rostedt
2017-02-27 16:52     ` [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in return probes Steven Rostedt (VMware)
2017-02-28  0:01       ` Masami Hiramatsu
2017-03-01 15:16       ` Naveen N. Rao
2017-02-22 13:53 ` [PATCH v2 4/5] perf: kretprobes: offset from reloc_sym if kernel supports it Naveen N. Rao
2017-02-23  9:10   ` Masami Hiramatsu
2017-02-23 11:37     ` [PATCH v3 1/2] perf: probe: generalize probe event file open routine Naveen N. Rao
2017-02-24 16:46       ` Masami Hiramatsu
2017-02-24 20:07         ` Arnaldo Carvalho de Melo
2017-03-01 15:12         ` Naveen N. Rao
2017-02-23 11:37     ` [PATCH v3 2/2] perf: kretprobes: offset from reloc_sym if kernel supports it Naveen N. Rao
2017-02-24 17:12       ` Masami Hiramatsu
2017-03-01 15:11         ` Naveen N. Rao
2017-02-23 19:16     ` [PATCH v2 4/5] " Naveen N. Rao
2017-02-24 17:29       ` Masami Hiramatsu
2017-02-24 20:11         ` Arnaldo Carvalho de Melo
2017-02-24 23:55           ` Masami Hiramatsu
2017-03-01 15:14             ` Naveen N. Rao
2017-03-02 17:55           ` Naveen N. Rao
2017-03-02 19:06             ` Arnaldo Carvalho de Melo
2017-03-02 17:55           ` [PATCH v4 1/3] perf: probe: factor out the ftrace README scanning Naveen N. Rao
2017-03-04  0:09             ` Masami Hiramatsu
2017-03-07 20:45             ` Steven Rostedt
2017-03-02 17:55           ` [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it Naveen N. Rao
2017-03-04  0:49             ` Masami Hiramatsu
2017-03-04  2:35               ` Masami Hiramatsu
2017-03-04  2:38                 ` Masami Hiramatsu
2017-03-04  4:34                 ` Masami Hiramatsu
2017-03-06 16:20                   ` Naveen N. Rao
2017-03-06 17:49                   ` Naveen N. Rao
2017-03-06 21:06                     ` Masami Hiramatsu
2017-03-07 10:47                       ` [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel Naveen N. Rao
2017-03-07 10:47                       ` Naveen N. Rao [this message]
2017-03-07 20:47                         ` [RESEND PATCH 1/6] trace/kprobes: fix check for kretprobe offset within function entry Steven Rostedt
2017-03-08  8:01                           ` Naveen N. Rao
2017-03-07 10:47                       ` [RESEND PATCH 2/6] powerpc: kretprobes: override default function entry offset Naveen N. Rao
2017-03-07 10:47                       ` [RESEND PATCH 3/6] perf: probe: factor out the ftrace README scanning Naveen N. Rao
2017-03-07 10:47                       ` [RESEND PATCH 4/6] perf: kretprobes: offset from reloc_sym if kernel supports it Naveen N. Rao
2017-03-07 10:47                       ` [PATCH 5/6] perf: probes: move ftrace README parsing logic into trace-event-parse.c Naveen N. Rao
2017-03-07 14:03                         ` Masami Hiramatsu
2017-03-07 14:29                           ` Naveen N. Rao
2017-03-07 15:51                         ` Masami Hiramatsu
2017-03-07 16:31                           ` Naveen N. Rao
2017-03-07 10:47                       ` [RESEND PATCH 6/6] perf: powerpc: choose local entry point with kretprobes Naveen N. Rao
2017-03-07 16:49                         ` [PATCH v2 " Naveen N. Rao
2017-03-06 15:04               ` [PATCH v4 2/3] perf: kretprobes: offset from reloc_sym if kernel supports it Naveen N. Rao
2017-03-06 21:14                 ` Masami Hiramatsu
2017-03-02 17:55           ` [PATCH v4 3/3] perf: powerpc: choose local entry point with kretprobes Naveen N. Rao
2017-03-04  0:50             ` Masami Hiramatsu
2017-02-22 13:53 ` [PATCH v2 5/5] " Naveen N. Rao

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=1bcc13b27c8338f7b12c160ac049029e2007cc99.1488881988.git.naveen.n.rao@linux.vnet.ibm.com \
    --to=naveen.n.rao@linux.vnet.ibm.com \
    --cc=acme@kernel.org \
    --cc=ananth@linux.vnet.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=mpe@ellerman.id.au \
    --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;
as well as URLs for NNTP newsgroup(s).