perf annotate: Validate addr in symbol__inc_addr_samples

This routine was checking only if the provided address was after
sym->end, not if it was before sym->start.

Fix that by checking for both and return in both cases -ERANGE, so that
tools can communicate this to the user properly, or if they chose so, to
abort.

This problem was reported previously but the fixes involved either doing
what was being done for the > end case, i.e. silently drop the sample,
returning 0, or aborting at this function, which is in a lib (or better,
is slated to be at some point) and shouldn't abort.

The 'report' tool already checks this value and uses pr_debug to warn
the user.

This patch makes the 'top' tool check it too and warn once per map where
such range problem takes place.
Reported-by: default avatarDavid Miller <davem@davemloft.net>
Reported-by: default avatarSorin Dumitru <dumitru.sorin87@gmail.com>
Reported-by: default avatarStephane Eranian <eranian@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/n/tip-lw8gs7p9i9nhldilo82tzpne@git.kernel.orgSigned-off-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
parent 8493fe1d
...@@ -42,6 +42,7 @@ ...@@ -42,6 +42,7 @@
#include "util/debug.h" #include "util/debug.h"
#include <assert.h> #include <assert.h>
#include <elf.h>
#include <fcntl.h> #include <fcntl.h>
#include <stdio.h> #include <stdio.h>
...@@ -59,6 +60,7 @@ ...@@ -59,6 +60,7 @@
#include <sys/prctl.h> #include <sys/prctl.h>
#include <sys/wait.h> #include <sys/wait.h>
#include <sys/uio.h> #include <sys/uio.h>
#include <sys/utsname.h>
#include <sys/mman.h> #include <sys/mman.h>
#include <linux/unistd.h> #include <linux/unistd.h>
...@@ -162,12 +164,40 @@ static void __zero_source_counters(struct hist_entry *he) ...@@ -162,12 +164,40 @@ static void __zero_source_counters(struct hist_entry *he)
symbol__annotate_zero_histograms(sym); symbol__annotate_zero_histograms(sym);
} }
static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip)
{
struct utsname uts;
int err = uname(&uts);
ui__warning("Out of bounds address found:\n\n"
"Addr: %" PRIx64 "\n"
"DSO: %s %c\n"
"Map: %" PRIx64 "-%" PRIx64 "\n"
"Symbol: %" PRIx64 "-%" PRIx64 " %c %s\n"
"Arch: %s\n"
"Kernel: %s\n"
"Tools: %s\n\n"
"Not all samples will be on the annotation output.\n\n"
"Please report to linux-kernel@vger.kernel.org\n",
ip, map->dso->long_name, dso__symtab_origin(map->dso),
map->start, map->end, sym->start, sym->end,
sym->binding == STB_GLOBAL ? 'g' :
sym->binding == STB_LOCAL ? 'l' : 'w', sym->name,
err ? "[unknown]" : uts.machine,
err ? "[unknown]" : uts.release, perf_version_string);
if (use_browser <= 0)
sleep(5);
map->erange_warned = true;
}
static void perf_top__record_precise_ip(struct perf_top *top, static void perf_top__record_precise_ip(struct perf_top *top,
struct hist_entry *he, struct hist_entry *he,
int counter, u64 ip) int counter, u64 ip)
{ {
struct annotation *notes; struct annotation *notes;
struct symbol *sym; struct symbol *sym;
int err;
if (he == NULL || he->ms.sym == NULL || if (he == NULL || he->ms.sym == NULL ||
((top->sym_filter_entry == NULL || ((top->sym_filter_entry == NULL ||
...@@ -189,9 +219,12 @@ static void perf_top__record_precise_ip(struct perf_top *top, ...@@ -189,9 +219,12 @@ static void perf_top__record_precise_ip(struct perf_top *top,
} }
ip = he->ms.map->map_ip(he->ms.map, ip); ip = he->ms.map->map_ip(he->ms.map, ip);
symbol__inc_addr_samples(sym, he->ms.map, counter, ip); err = symbol__inc_addr_samples(sym, he->ms.map, counter, ip);
pthread_mutex_unlock(&notes->lock); pthread_mutex_unlock(&notes->lock);
if (err == -ERANGE && !he->ms.map->erange_warned)
ui__warn_map_erange(he->ms.map, sym, ip);
} }
static void perf_top__show_details(struct perf_top *top) static void perf_top__show_details(struct perf_top *top)
......
...@@ -64,8 +64,8 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map, ...@@ -64,8 +64,8 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map, addr)); pr_debug3("%s: addr=%#" PRIx64 "\n", __func__, map->unmap_ip(map, addr));
if (addr > sym->end) if (addr < sym->start || addr > sym->end)
return 0; return -ERANGE;
offset = addr - sym->start; offset = addr - sym->start;
h = annotation__histogram(notes, evidx); h = annotation__histogram(notes, evidx);
......
...@@ -38,6 +38,7 @@ void map__init(struct map *self, enum map_type type, ...@@ -38,6 +38,7 @@ void map__init(struct map *self, enum map_type type,
RB_CLEAR_NODE(&self->rb_node); RB_CLEAR_NODE(&self->rb_node);
self->groups = NULL; self->groups = NULL;
self->referenced = false; self->referenced = false;
self->erange_warned = false;
} }
struct map *map__new(struct list_head *dsos__list, u64 start, u64 len, struct map *map__new(struct list_head *dsos__list, u64 start, u64 len,
......
...@@ -33,6 +33,7 @@ struct map { ...@@ -33,6 +33,7 @@ struct map {
u64 end; u64 end;
u8 /* enum map_type */ type; u8 /* enum map_type */ type;
bool referenced; bool referenced;
bool erange_warned;
u32 priv; u32 priv;
u64 pgoff; u64 pgoff;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment