On Wed, 20 Jan 2021 12:24:15 +0900 Masami Hiramatsu <[email protected]> wrote:
> Hi, > > On Tue, 19 Jan 2021 10:41:06 -0500 > Steven Rostedt <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> > > > --- > > > 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 <[email protected]> -- Masami Hiramatsu <[email protected]>
