linux-kernel - Re: [PATCH v3] trace/kprobe: Display the actual notrace function when rejecting a probe

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: <20231214080210.99ff94d956307b036208520d@kernel.org>
Date:   Thu, 14 Dec 2023 08:02:10 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Naveen N Rao <naveen@...nel.org>
Cc:     <linux-kernel@...r.kernel.org>,
        <linux-trace-kernel@...r.kernel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Srikar Dronamraju <srikar@...ux.ibm.com>
Subject: Re: [PATCH v3] trace/kprobe: Display the actual notrace function
 when rejecting a probe

On Wed, 13 Dec 2023 20:09:14 +0530
Naveen N Rao <naveen@...nel.org> wrote:

> Trying to probe update_sd_lb_stats() using perf results in the below
> message in the kernel log:
>   trace_kprobe: Could not probe notrace function _text
> 
> This is because 'perf probe' specifies the kprobe location as an offset
> from '_text':
>   $ sudo perf probe -D update_sd_lb_stats
>   p:probe/update_sd_lb_stats _text+1830728
> 
> However, the error message is misleading and doesn't help convey the
> actual notrace function that is being probed. Fix this by looking up the
> actual function name that is being probed. With this fix, we now get the
> below message in the kernel log:
>   trace_kprobe: Could not probe notrace function update_sd_lb_stats.constprop.0
> 
> Signed-off-by: Naveen N Rao <naveen@...nel.org>
> ---
> v3: Remove tk parameter from within_notrace_func() as suggested by 
> Masami
> 
>  kernel/trace/trace_kprobe.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 3d7a180a8427..dc36c6ed6131 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -449,9 +449,8 @@ static bool __within_notrace_func(unsigned long addr)
>  	return !ftrace_location_range(addr, addr + size - 1);
>  }
>  
> -static bool within_notrace_func(struct trace_kprobe *tk)
> +static bool within_notrace_func(unsigned long addr)
>  {
> -	unsigned long addr = trace_kprobe_address(tk);
>  	char symname[KSYM_NAME_LEN], *p;
>  
>  	if (!__within_notrace_func(addr))
> @@ -471,12 +470,14 @@ static bool within_notrace_func(struct trace_kprobe *tk)
>  	return true;
>  }
>  #else
> -#define within_notrace_func(tk)	(false)
> +#define within_notrace_func(addr)	(false)
>  #endif
>  
>  /* Internal register function - just handle k*probes and flags */
>  static int __register_trace_kprobe(struct trace_kprobe *tk)
>  {
> +	unsigned long addr = trace_kprobe_address(tk);
> +	char symname[KSYM_NAME_LEN];
>  	int i, ret;
>  
>  	ret = security_locked_down(LOCKDOWN_KPROBES);
> @@ -486,9 +487,9 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
>  	if (trace_kprobe_is_registered(tk))
>  		return -EINVAL;
>  
> -	if (within_notrace_func(tk)) {
> +	if (within_notrace_func(addr)) {
>  		pr_warn("Could not probe notrace function %s\n",
> -			trace_kprobe_symbol(tk));
> +			lookup_symbol_name(addr, symname) ? trace_kprobe_symbol(tk) : symname);

Can we just use %ps and (void *)trace_kprobe_address(tk) here?

That will be simpler.

Thank you,

>  		return -EINVAL;
>  	}
>  
> 
> base-commit: 4758560fa268cecfa1144f015aa9f2525d164b7e
> -- 
> 2.43.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>