[PATCH v2] trace/kprobe: Display the actual notrace function when rejecting a probe

[PATCH v2] trace/kprobe: Display the actual notrace function when rejecting a probe

[Naveen N Rao]

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@kernel.org>
---
v2: Update within_notrace_func() stub macro with the second parameter to 
fix the build error reported by the kernel test robot.

- Naveen

 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 74adb82331dd81..2d695db5425e7c 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(struct trace_kprobe *tk, 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(tk, 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(tk, addr)) {
 		pr_warn("Could not probe notrace function %s\n",
-			trace_kprobe_symbol(tk));
+			lookup_symbol_name(addr, symname) ? trace_kprobe_symbol(tk) : symname);
 		return -EINVAL;
 	}
 

base-commit: e46ad59233cf16daf4f3b9dd080003f01ac940fe
-- 
2.40.1

Re: [PATCH v2] trace/kprobe: Display the actual notrace function when rejecting a probe

[Masami Hiramatsu]

Hi Naveen,

Sorry I missed this patch.

On Fri,  9 Jun 2023 13:28:09 +0530
Naveen N Rao <naveen@kernel.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@kernel.org>
> ---
> v2: Update within_notrace_func() stub macro with the second parameter to 
> fix the build error reported by the kernel test robot.
> 
> - Naveen
> 
>  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 74adb82331dd81..2d695db5425e7c 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(struct trace_kprobe *tk, 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(tk, addr)	(false)
>  #endif

Is this for avoiding redundant calling the trace_kprobe_address(tk)? If so,
please pass only 'addr' to the function since 'tk' is only used for calling
trace_kprobe_address(tk) in the within_notrace_func(). :)

Thank you,

>  
>  /* 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(tk, addr)) {
>  		pr_warn("Could not probe notrace function %s\n",
> -			trace_kprobe_symbol(tk));
> +			lookup_symbol_name(addr, symname) ? trace_kprobe_symbol(tk) : symname);
>  		return -EINVAL;
>  	}
>  
> 
> base-commit: e46ad59233cf16daf4f3b9dd080003f01ac940fe
> -- 
> 2.40.1
> 


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