Re: [PATCH v3] trace/kprobe: Display the actual notrace function when rejecting a probe
On Thu, Dec 14, 2023 at 08:02:10AM +0900, Masami Hiramatsu wrote:
> On Wed, 13 Dec 2023 20:09:14 +0530
> Naveen N Rao 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
> > ---
> > 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.
Indeed - that is much simpler. v4 on its way...
Thanks!
- Naveen
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 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
> ---
> 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)
