Re: [PATCH 1/2] perf/probe: Report possible permission error for map__load failure

[Masami Hiramatsu]

On Sun, 6 Jun 2021 00:11:31 -0700
Namhyung Kim <namhyung@xxxxxxxxxx> wrote:

> Hi Arnaldo and Masami,
> 
> On Fri, Jun 4, 2021 at 12:18 PM Arnaldo Carvalho de Melo
> <acme@xxxxxxxxxx> wrote:
> >
> > Em Sat, Jun 05, 2021 at 12:30:58AM +0900, Masami Hiramatsu escreveu:
> > > Report possible permission error including kptr_restrict setting
> > > for map__load() failure. This can happen when non-superuser runs
> > > perf probe.
> > >
> > > With this patch, perf probe shows the following message.
> > >
> > >  $ perf probe vfs_read
> > >  Failed to load symbols from /proc/kallsyms
> > >  Please ensure you can read the /proc/kallsyms symbol addresses.
> > >  If the /proc/sys/kernel/kptr_restrict is '2', you can not read
> > >  kernel symbol address even if you are a superuser. Please change
> > >  it to '1'. If kptr_restrict is '1', the superuser can read the
> > >  symbol addresses.
> > >  In that case, please run this command again with sudo.
> > >    Error: Failed to add events.
> 
> Maybe we can read the kptr_restrict file and (e)uid to suggest
> the specific message for the situation only.

Agreed, and that should be done in map__load(), since it returns -1
for any error. Caller needs to estimate the reason.

If each caller does it, those have to repeat same estimation and
similar messages in different place. Maybe we need to have a global
error and hint object so that caller can show it when it detects
an error.

Thank you,


> Thanks,
> Namhyung
> 
> >
> >
> > > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > > ---
> > >  tools/perf/util/probe-event.c |   25 ++++++++++++++++++++++---
> > >  1 file changed, 22 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index 3a7649835ec9..8fe179d671c3 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -2936,7 +2936,7 @@ static int find_probe_functions(struct map *map, char *name,
> > >       bool cut_version = true;
> > >
> > >       if (map__load(map) < 0)
> > > -             return 0;
> > > +             return -EACCES; /* Possible permission error to load symbols */
> > >
> > >       /* If user gives a version, don't cut off the version from symbols */
> > >       if (strchr(name, '@'))
> > > @@ -2975,6 +2975,17 @@ void __weak arch__fix_tev_from_maps(struct perf_probe_event *pev __maybe_unused,
> > >                               struct map *map __maybe_unused,
> > >                               struct symbol *sym __maybe_unused) { }
> > >
> > > +
> > > +static void pr_kallsyms_access_error(void)
> > > +{
> > > +     pr_err("Please ensure you can read the /proc/kallsyms symbol addresses.\n"
> > > +            "If the /proc/sys/kernel/kptr_restrict is '2', you can not read\n"
> > > +            "kernel symbol address even if you are a superuser. Please change\n"
> > > +            "it to '1'. If kptr_restrict is '1', the superuser can read the\n"
> > > +            "symbol addresses.\n"
> > > +            "In that case, please run this command again with sudo.\n");
> > > +}
> > > +
> > >  /*
> > >   * Find probe function addresses from map.
> > >   * Return an error or the number of found probe_trace_event
> > > @@ -3011,8 +3022,16 @@ static int find_probe_trace_events_from_map(struct perf_probe_event *pev,
> > >        */
> > >       num_matched_functions = find_probe_functions(map, pp->function, syms);
> > >       if (num_matched_functions <= 0) {
> > > -             pr_err("Failed to find symbol %s in %s\n", pp->function,
> > > -                     pev->target ? : "kernel");
> > > +             if (num_matched_functions == -EACCES) {
> > > +                     pr_err("Failed to load symbols from %s\n",
> > > +                            pev->target ?: "/proc/kallsyms");
> > > +                     if (pev->target)
> > > +                             pr_err("Please ensure the file is not stripped.\n");
> > > +                     else
> > > +                             pr_kallsyms_access_error();
> > > +             } else
> > > +                     pr_err("Failed to find symbol %s in %s\n", pp->function,
> > > +                             pev->target ? : "kernel");
> > >               ret = -ENOENT;
> > >               goto out;
> > >       } else if (num_matched_functions > probe_conf.max_probes) {
> > >
> >
> > --
> >
> > - Arnaldo


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>