perf annotate: Fix non-null terminated buffer returned by readlink()

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@google.com>
Acked-by: Ian Rogers <irogers@google.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Mark Drayton <mbd@fb.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Song Liu <songliubraving@fb.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lore.kernel.org/lkml/20190729205750.193289-1-nums@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 76bfb4a..0a1fcf7 100644 (file)
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1621,6 +1621,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))
@@ -1649,10 +1650,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