* [PATCH] tracing: precise log info for kretprobe addr err
@ 2021-01-19 15:10 Jianlin Lv
2021-01-19 15:41 ` Steven Rostedt
0 siblings, 1 reply; 4+ messages in thread
From: Jianlin Lv @ 2021-01-19 15:10 UTC (permalink / raw)
To: rostedt, mingo; +Cc: Jianlin.Lv, linux-kernel
When trying to create kretprobe with the wrong function symbol in tracefs;
The error is triggered in the register_trace_kprobe() and recorded as
FAIL_REG_PROBE issue,
Example:
$ cd /sys/kernel/debug/tracing
$ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
bash: echo: write error: Invalid argument
$ cat error_log
[142797.347877] trace_kprobe: error: Failed to register probe event
Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
^
This error can be detected in the parameter parsing stage, the effect of
applying this patch is as follows:
$ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
bash: echo: write error: Invalid argument
$ cat error_log
[415.89]trace_kprobe: error: Retprobe address must be an function entry
Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
^
Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
---
kernel/trace/trace_kprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771..3dfd1b6711a3 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
flags |= TPARG_FL_RETURN;
if (kprobe_on_func_entry(NULL, symbol, offset))
flags |= TPARG_FL_FENTRY;
- if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
+ if (is_return && !(flags & TPARG_FL_FENTRY)) {
trace_probe_log_err(0, BAD_RETPROBE);
goto parse_error;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] tracing: precise log info for kretprobe addr err
2021-01-19 15:10 [PATCH] tracing: precise log info for kretprobe addr err Jianlin Lv
@ 2021-01-19 15:41 ` Steven Rostedt
2021-01-20 3:24 ` Masami Hiramatsu
0 siblings, 1 reply; 4+ messages in thread
From: Steven Rostedt @ 2021-01-19 15:41 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Jianlin Lv, mingo, linux-kernel
Masami,
Looks fine to me. What do you think?
-- Steve
On Tue, 19 Jan 2021 23:10:14 +0800
Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> When trying to create kretprobe with the wrong function symbol in tracefs;
> The error is triggered in the register_trace_kprobe() and recorded as
> FAIL_REG_PROBE issue,
>
> Example:
> $ cd /sys/kernel/debug/tracing
> $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> bash: echo: write error: Invalid argument
> $ cat error_log
> [142797.347877] trace_kprobe: error: Failed to register probe event
> Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> ^
>
> This error can be detected in the parameter parsing stage, the effect of
> applying this patch is as follows:
>
> $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> bash: echo: write error: Invalid argument
> $ cat error_log
> [415.89]trace_kprobe: error: Retprobe address must be an function entry
> Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> ^
>
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
> kernel/trace/trace_kprobe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index e6fba1798771..3dfd1b6711a3 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
> flags |= TPARG_FL_RETURN;
> if (kprobe_on_func_entry(NULL, symbol, offset))
> flags |= TPARG_FL_FENTRY;
> - if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
> + if (is_return && !(flags & TPARG_FL_FENTRY)) {
> trace_probe_log_err(0, BAD_RETPROBE);
> goto parse_error;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tracing: precise log info for kretprobe addr err
2021-01-19 15:41 ` Steven Rostedt
@ 2021-01-20 3:24 ` Masami Hiramatsu
2021-01-20 4:14 ` Masami Hiramatsu
0 siblings, 1 reply; 4+ messages in thread
From: Masami Hiramatsu @ 2021-01-20 3:24 UTC (permalink / raw)
To: Steven Rostedt; +Cc: Jianlin Lv, mingo, linux-kernel
Hi,
On Tue, 19 Jan 2021 10:41:06 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> Masami,
>
> Looks fine to me. What do you think?
Agreed. Since register_kretprobe() checks the address by kprobe_on_func_entry(),
if it is not passed, it should always fail to register at last.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Thank you!
>
> -- Steve
>
>
> On Tue, 19 Jan 2021 23:10:14 +0800
> Jianlin Lv <Jianlin.Lv@arm.com> wrote:
>
> > When trying to create kretprobe with the wrong function symbol in tracefs;
> > The error is triggered in the register_trace_kprobe() and recorded as
> > FAIL_REG_PROBE issue,
> >
> > Example:
> > $ cd /sys/kernel/debug/tracing
> > $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> > bash: echo: write error: Invalid argument
> > $ cat error_log
> > [142797.347877] trace_kprobe: error: Failed to register probe event
> > Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> > ^
> >
> > This error can be detected in the parameter parsing stage, the effect of
> > applying this patch is as follows:
> >
> > $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> > bash: echo: write error: Invalid argument
> > $ cat error_log
> > [415.89]trace_kprobe: error: Retprobe address must be an function entry
> > Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> > ^
> >
> > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > ---
> > kernel/trace/trace_kprobe.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > index e6fba1798771..3dfd1b6711a3 100644
> > --- a/kernel/trace/trace_kprobe.c
> > +++ b/kernel/trace/trace_kprobe.c
> > @@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
> > flags |= TPARG_FL_RETURN;
> > if (kprobe_on_func_entry(NULL, symbol, offset))
> > flags |= TPARG_FL_FENTRY;
> > - if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
> > + if (is_return && !(flags & TPARG_FL_FENTRY)) {
> > trace_probe_log_err(0, BAD_RETPROBE);
> > goto parse_error;
> > }
>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] tracing: precise log info for kretprobe addr err
2021-01-20 3:24 ` Masami Hiramatsu
@ 2021-01-20 4:14 ` Masami Hiramatsu
0 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2021-01-20 4:14 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Steven Rostedt, Jianlin Lv, mingo, linux-kernel
On Wed, 20 Jan 2021 12:24:15 +0900
Masami Hiramatsu <mhiramat@kernel.org> wrote:
> Hi,
>
> On Tue, 19 Jan 2021 10:41:06 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > Masami,
> >
> > Looks fine to me. What do you think?
>
> Agreed. Since register_kretprobe() checks the address by kprobe_on_func_entry(),
> if it is not passed, it should always fail to register at last.
>
> Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Oops, sorry I missed one case.
Since kprobe_events can accept a symbol with module name (MOD:symbol)
If the given symbol is on a module which is not loaded,
kprobe_on_func_entry(NULL, symbol, offset) may return false too.
(But that kretprobe must be enabled after the target module is loaded)
To confirm it correctly, add !strchr(symbol, ':') there to check
it is really bad symbol or not.
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5fff39541b8a..0170f8e6430b 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
flags |= TPARG_FL_RETURN;
if (kprobe_on_func_entry(NULL, symbol, offset))
flags |= TPARG_FL_FENTRY;
- if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
+ if (!strchr(symbol, ':') && is_return && !(flags & TPARG_FL_FENTRY)) {
trace_probe_log_err(0, BAD_RETPROBE);
goto parse_error;
}
Thank you,
>
> Thank you!
>
> >
> > -- Steve
> >
> >
> > On Tue, 19 Jan 2021 23:10:14 +0800
> > Jianlin Lv <Jianlin.Lv@arm.com> wrote:
> >
> > > When trying to create kretprobe with the wrong function symbol in tracefs;
> > > The error is triggered in the register_trace_kprobe() and recorded as
> > > FAIL_REG_PROBE issue,
> > >
> > > Example:
> > > $ cd /sys/kernel/debug/tracing
> > > $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> > > bash: echo: write error: Invalid argument
> > > $ cat error_log
> > > [142797.347877] trace_kprobe: error: Failed to register probe event
> > > Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> > > ^
> > >
> > > This error can be detected in the parameter parsing stage, the effect of
> > > applying this patch is as follows:
> > >
> > > $ echo 'r:myprobe ERROR_SYMBOL_XXX ret=%x0' >> kprobe_events
> > > bash: echo: write error: Invalid argument
> > > $ cat error_log
> > > [415.89]trace_kprobe: error: Retprobe address must be an function entry
> > > Command: r:myprobe ERROR_SYMBOL_XXX ret=%x0
> > > ^
> > >
> > > Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> > > ---
> > > kernel/trace/trace_kprobe.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> > > index e6fba1798771..3dfd1b6711a3 100644
> > > --- a/kernel/trace/trace_kprobe.c
> > > +++ b/kernel/trace/trace_kprobe.c
> > > @@ -830,7 +830,7 @@ static int trace_kprobe_create(int argc, const char *argv[])
> > > flags |= TPARG_FL_RETURN;
> > > if (kprobe_on_func_entry(NULL, symbol, offset))
> > > flags |= TPARG_FL_FENTRY;
> > > - if (offset && is_return && !(flags & TPARG_FL_FENTRY)) {
> > > + if (is_return && !(flags & TPARG_FL_FENTRY)) {
> > > trace_probe_log_err(0, BAD_RETPROBE);
> > > goto parse_error;
> > > }
> >
>
>
> --
> Masami Hiramatsu <mhiramat@kernel.org>
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-20 4:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 15:10 [PATCH] tracing: precise log info for kretprobe addr err Jianlin Lv
2021-01-19 15:41 ` Steven Rostedt
2021-01-20 3:24 ` Masami Hiramatsu
2021-01-20 4:14 ` Masami Hiramatsu
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).