linux-kernel - Re: [PATCH v2] tracing: precise log info for kretprobe addr err

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210120112004.4b9ff1df@gandalf.local.home>
Date:   Wed, 20 Jan 2021 11:20:04 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Jianlin Lv <Jianlin.Lv@....com>
Cc:     mingo@...hat.com, linux-kernel@...r.kernel.org,
        Masami Hiramatsu <mhiramat@...nel.org>
Subject: Re: [PATCH v2] tracing: precise log info for kretprobe addr err


You forgot to include Masami on Cc again. Masami maintains kprobes. Please
include him on any updates, as he needs to review them, and give his
acknowledgment before acceptance.

I need to update MAINTAINERS files to include trace_kprobe under KPROBES.
And I also don't see any UPROBES section there. That needs to be done as
well.

On Wed, 20 Jan 2021 23:56:44 +0800
Jianlin Lv <Jianlin.Lv@....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@....com>
> 
> v2:add !strchr(symbol, ':') to check really bad symbol or not.

Also, the "changes since" section should be below the "---" so that they
don't get pulled into the commit.

Thanks!

-- Steve


> ---
>  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..bce63d5ecaec 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;
>  		}