diff options
author | Numfor Mbiziwo-Tiapo <nums@google.com> | 2019-07-29 13:57:50 -0700 |
---|---|---|
committer | Arnaldo Carvalho de Melo <acme@redhat.com> | 2020-07-09 12:36:50 -0300 |
commit | b39730a6633d630b2a7bc61e597a0c671eb753b4 (patch) | |
tree | 966d665d4ada45f93db54e19425682d564bf3307 /tools/perf/util | |
parent | c8f6ae1fb28d74ef7a0966477f61d1c2c8ee2e1e (diff) |
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>
Diffstat (limited to 'tools/perf/util')
-rw-r--r-- | tools/perf/util/annotate.c | 15 |
1 files changed, 11 insertions, 4 deletions
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 76bfb4a9d94e..0a1fcf787538 100644 --- 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 |