Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1586167 > unrolled thread

[PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> - 2017-02-22 15:00 +0100
    Re: [PATCH v2 3/5] trace/kprobes: allow return probes with offsets  and absolute addresses Steven Rostedt <rostedt@goodmis.org> - 2017-02-27 17:50 +0100
      [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in  return probes "Steven Rostedt (VMware)" <rostedt@goodmis.org> - 2017-02-27 18:10 +0100
        Re: [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset  in return probes Masami Hiramatsu <mhiramat@kernel.org> - 2017-02-28 01:30 +0100

#1586167 — [PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses

Since the kernel includes many non-global functions with same names, we
will need to use offsets from other symbols (typically _text/_stext) or
absolute addresses to place return probes on specific functions. Also,
the core register_kretprobe() API never forbid use of offsets or
absolute addresses with kretprobes.

Allow its use with the trace infrastructure. To distinguish kernels that
support this, update ftrace README to explicitly call this out.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 kernel/trace/trace.c        | 1 +
 kernel/trace/trace_kprobe.c | 8 --------
 2 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d7449783987a..ababe56b3e8f 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4362,6 +4362,7 @@ static const char readme_msg[] =
 	"\t           -:[<group>/]<event>\n"
 #ifdef CONFIG_KPROBE_EVENT
 	"\t    place: [<module>:]<symbol>[+<offset>]|<memaddr>\n"
+  "place (kretprobe): [<module>:]<symbol>[+<offset>]|<memaddr>\n"
 #endif
 #ifdef CONFIG_UPROBE_EVENT
 	"\t    place: <path>:<offset>\n"
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7ad9e53ad174..2768cb60ebd7 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -679,10 +679,6 @@ static int create_trace_kprobe(int argc, char **argv)
 		return -EINVAL;
 	}
 	if (isdigit(argv[1][0])) {
-		if (is_return) {
-			pr_info("Return probe point must be a symbol.\n");
-			return -EINVAL;
-		}
 		/* an address specified */
 		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
 		if (ret) {
@@ -698,10 +694,6 @@ static int create_trace_kprobe(int argc, char **argv)
 			pr_info("Failed to parse symbol.\n");
 			return ret;
 		}
-		if (offset && is_return) {
-			pr_info("Return probe must be used without offset.\n");
-			return -EINVAL;
-		}
 	}
 	argc -= 2; argv += 2;
 
-- 
2.11.0

[toc] | [next] | [standalone]

#1588806 — Re: [PATCH v2 3/5] trace/kprobes: allow return probes with offsets and absolute addresses

On Wed, 22 Feb 2017 19:23:39 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> Since the kernel includes many non-global functions with same names, we
> will need to use offsets from other symbols (typically _text/_stext) or
> absolute addresses to place return probes on specific functions. Also,
> the core register_kretprobe() API never forbid use of offsets or
> absolute addresses with kretprobes.
> 
> Allow its use with the trace infrastructure. To distinguish kernels that
> support this, update ftrace README to explicitly call this out.
> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
>  kernel/trace/trace.c        | 1 +
>  kernel/trace/trace_kprobe.c | 8 --------
>  2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index d7449783987a..ababe56b3e8f 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4362,6 +4362,7 @@ static const char readme_msg[] =
>  	"\t           -:[<group>/]<event>\n"
>  #ifdef CONFIG_KPROBE_EVENT
>  	"\t    place: [<module>:]<symbol>[+<offset>]|<memaddr>\n"
> +  "place (kretprobe): [<module>:]<symbol>[+<offset>]|<memaddr>\n"
>  #endif
>  #ifdef CONFIG_UPROBE_EVENT
>  	"\t    place: <path>:<offset>\n"
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 7ad9e53ad174..2768cb60ebd7 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -679,10 +679,6 @@ static int create_trace_kprobe(int argc, char **argv)
>  		return -EINVAL;
>  	}
>  	if (isdigit(argv[1][0])) {
> -		if (is_return) {
> -			pr_info("Return probe point must be a symbol.\n");
> -			return -EINVAL;
> -		}
>  		/* an address specified */
>  		ret = kstrtoul(&argv[1][0], 0, (unsigned long *)&addr);
>  		if (ret) {
> @@ -698,10 +694,6 @@ static int create_trace_kprobe(int argc, char **argv)
>  			pr_info("Failed to parse symbol.\n");
>  			return ret;
>  		}
> -		if (offset && is_return) {
> -			pr_info("Return probe must be used without offset.\n");
> -			return -EINVAL;
> -		}

I understand that your retprobes will now have an offset, but I'm
worried we are removing informative errors. For those archs that don't
allow an offset, will we still get the error telling users that offsets
are not allowed?

I don't want to lose informative error handling.

-- Steve


>  	}
>  	argc -= 2; argv += 2;
>  

[toc] | [prev] | [next] | [standalone]

#1588818 — [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in return probes

Let's not remove the warning about offsets and return probes when the
offset is invalid.

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3f4f788..f626235 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -695,6 +695,11 @@ static int create_trace_kprobe(int argc, char **argv)
 			pr_info("Failed to parse symbol.\n");
 			return ret;
 		}
+		if (offset && is_return &&
+		    !arch_function_offset_within_entry(offset)) {
+			pr_info("Given offset is not valid for return probe.\n");
+			return -EINVAL;
+		}
 	}
 	argc -= 2; argv += 2;
 

[toc] | [prev] | [next] | [standalone]

#1589064 — Re: [PATCH v2 3.5/5] trace/kprobes: Add back warning about offset in return probes

On Mon, 27 Feb 2017 11:52:04 -0500
"Steven Rostedt (VMware)" <rostedt@goodmis.org> wrote:

> Let's not remove the warning about offsets and return probes when the
> offset is invalid.

Agreed, This looks good to me.

Acked-by: Masami Hiramatsu <mhiramat@kernel.org>

Thanks!

> 
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3f4f788..f626235 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -695,6 +695,11 @@ static int create_trace_kprobe(int argc, char **argv)
>  			pr_info("Failed to parse symbol.\n");
>  			return ret;
>  		}
> +		if (offset && is_return &&
> +		    !arch_function_offset_within_entry(offset)) {
> +			pr_info("Given offset is not valid for return probe.\n");
> +			return -EINVAL;
> +		}
>  	}
>  	argc -= 2; argv += 2;
>  


-- 
Masami Hiramatsu <mhiramat@kernel.org>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web