linux-kernel - Re: [PATCH 1/2] perf/probe: Report possible permission error for map_

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]

Date:   Mon, 7 Jun 2021 09:00:34 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Ravi Bangoria <ravi.bangoria@...ux.ibm.com>,
        Jiri Olsa <jolsa@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        aneesh.kumar@...ux.ibm.com, Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, Ian Rogers <irogers@...gle.com>
Subject: Re: [PATCH 1/2] perf/probe: Report possible permission error for
 map__load failure

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

> Hi Arnaldo and Masami,
> 
> On Fri, Jun 4, 2021 at 12:18 PM Arnaldo Carvalho de Melo
> <acme@...nel.org> 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@...nel.org>
> > > ---
> > >  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@...nel.org>