[PATCH v4] tracing: precise log info for kretprobe addr err

[PATCH v4] tracing: precise log info for kretprobe addr err

[Jianlin Lv]

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
                       ^
v2 changes:
- Added !strchr(symbol, ':') to check whether symbol is really bad
  or from a module.

Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
---
v2: added !strchr(symbol, ':') to check really bad symbol or from module.
v4: added changelog and code comments.
---
 kernel/trace/trace_kprobe.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index e6fba1798771..384208a38f82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -830,7 +830,8 @@ 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)) {
+		/* Check whether symbol is really bad or from a module */
+		if (!strchr(symbol, ':') && is_return && !(flags & TPARG_FL_FENTRY)) {
 			trace_probe_log_err(0, BAD_RETPROBE);
 			goto parse_error;
 		}
-- 
2.25.1

Re: [PATCH v4] tracing: precise log info for kretprobe addr err

[Masami Hiramatsu]

Hi Jianlin,

Now as you reported, this must be fixed with actual bug which prevent
kretprobe events on unloaded module. I'll send another one which also
fix this issue.

Thank you,

On Wed, 27 Jan 2021 23:15:07 +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
>                        ^
> v2 changes:
> - Added !strchr(symbol, ':') to check whether symbol is really bad
>   or from a module.
> 
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
> v2: added !strchr(symbol, ':') to check really bad symbol or from module.
> v4: added changelog and code comments.
> ---
>  kernel/trace/trace_kprobe.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index e6fba1798771..384208a38f82 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -830,7 +830,8 @@ 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)) {
> +		/* Check whether symbol is really bad or from a module */
> +		if (!strchr(symbol, ':') && is_return && !(flags & TPARG_FL_FENTRY)) {
>  			trace_probe_log_err(0, BAD_RETPROBE);
>  			goto parse_error;
>  		}
> -- 
> 2.25.1
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>