linux-kernel - Re: [PATCH v2] Fix annotate.c use of uninitialized value error

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAP-5=fW8k6YWBYno2RWV5_mojn-0crvmPcLynKGBO_3WMCXfEA@mail.gmail.com>
Date:   Fri, 25 Oct 2019 15:11:02 -0700
From:   Ian Rogers <irogers@...gle.com>
To:     Jiri Olsa <jolsa@...hat.com>
Cc:     Numfor Mbiziwo-Tiapo <nums@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Song Liu <songliubraving@...com>, mbd@...com,
        LKML <linux-kernel@...r.kernel.org>,
        Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH v2] Fix annotate.c use of uninitialized value error

It looks like this wasn't merged to tip. Does anything need addressing
to get it merged?

Thanks,
Ian

On Wed, Aug 7, 2019 at 4:32 AM Jiri Olsa <jolsa@...hat.com> wrote:
>
> On Mon, Jul 29, 2019 at 01:57:50PM -0700, Numfor Mbiziwo-Tiapo wrote:
> > Our local MSAN (Memory Sanitizer) build of perf throws a warning
> > that comes from the "dso__disassemble_filename" function in
> > "tools/perf/util/annotate.c" when running perf record.
> >
> > The warning stems from the call to readlink, in which "build_id_path"
> > was being read into "linkname". Since readlink does not null terminate,
> > an uninitialized memory access would later occur when "linkname" is
> > passed into the strstr function. This is simply fixed by null-terminating
> > "linkname" after the call to readlink.
> >
> > To reproduce this warning, build perf by running:
> > make -C tools/perf CLANG=1 CC=clang EXTRA_CFLAGS="-fsanitize=memory\
> >  -fsanitize-memory-track-origins"
> >
> > (Additionally, llvm might have to be installed and clang might have to
> > be specified as the compiler - export CC=/usr/bin/clang)
> >
> > then running:
> > tools/perf/perf record -o - ls / | tools/perf/perf --no-pager annotate\
> >  -i - --stdio
> >
> > Please see the cover letter for why false positive warnings may be
> > generated.
> >
> > Signed-off-by: Numfor Mbiziwo-Tiapo <nums@...gle.com>
>
> Acked-by: Jiri Olsa <jolsa@...nel.org>
>
> thanks,
> jirka
>
> > ---
> >  tools/perf/util/annotate.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 70de8f6b3aee..e1b075b52dce 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -1627,6 +1627,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
> >       char *build_id_filename;
> >       char *build_id_path = NULL;
> >       char *pos;
> > +     int len;
> >
> >       if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
> >           !dso__is_kcore(dso))
> > @@ -1655,10 +1656,16 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
> >       if (pos && strlen(pos) < SBUILD_ID_SIZE - 2)
> >               dirname(build_id_path);
> >
> > -     if (dso__is_kcore(dso) ||
> > -         readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
> > -         strstr(linkname, DSO__NAME_KALLSYMS) ||
> > -         access(filename, R_OK)) {
> > +     if (dso__is_kcore(dso))
> > +             goto fallback;
> > +
> > +     len = readlink(build_id_path, linkname, sizeof(linkname) - 1);
> > +     if (len < 0)
> > +             goto fallback;
> > +
> > +     linkname[len] = '\0';
> > +     if (strstr(linkname, DSO__NAME_KALLSYMS) ||
> > +             access(filename, R_OK)) {
> >  fallback:
> >               /*
> >                * If we don't have build-ids or the build-id file isn't in the
> > --
> > 2.22.0.709.g102302147b-goog
> >