Re: [PATCH] tracing: precise log info for kretprobe addr err
On Wed, 20 Jan 2021 12:24:15 +0900 Masami Hiramatsu wrote: > Hi, > > On Tue, 19 Jan 2021 10:41:06 -0500 > Steven Rostedt 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 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 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 > > > --- > > > 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 -- Masami Hiramatsu
Re: [PATCH] tracing: precise log info for kretprobe addr err
Hi, On Tue, 19 Jan 2021 10:41:06 -0500 Steven Rostedt 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 Thank you! > > -- Steve > > > On Tue, 19 Jan 2021 23:10:14 +0800 > Jianlin Lv 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 > > --- > > 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
Re: [PATCH] tracing: precise log info for kretprobe addr err
Masami, Looks fine to me. What do you think? -- Steve On Tue, 19 Jan 2021 23:10:14 +0800 Jianlin Lv 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 > --- > 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; > }
[PATCH] tracing: precise log info for kretprobe addr err
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 --- 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