From 7522060c95395f479ee4a6af3bbf9e097e92e48f Mon Sep 17 00:00:00 2001 From: Ingo Molnar Date: Thu, 18 Jun 2009 08:00:17 +0200 Subject: [PATCH] perf report: Add validation of call-chain entries Add boundary checks for call-chain events. In case of corrupted entries we could crash otherwise. Cc: Peter Zijlstra Cc: Mike Galbraith Cc: Paul Mackerras Cc: Arnaldo Carvalho de Melo LKML-Reference: Signed-off-by: Ingo Molnar --- include/linux/perf_counter.h | 20 +++++----- tools/perf/builtin-report.c | 74 ++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h index eccae437fe37..a7d3a61a59b7 100644 --- a/include/linux/perf_counter.h +++ b/include/linux/perf_counter.h @@ -337,6 +337,16 @@ enum perf_event_type { */ }; +#define MAX_STACK_DEPTH 255 + +struct perf_callchain_entry { + __u16 nr; + __u16 hv; + __u16 kernel; + __u16 user; + __u64 ip[MAX_STACK_DEPTH]; +}; + #ifdef __KERNEL__ /* * Kernel-internal data types and definitions: @@ -652,16 +662,6 @@ extern void perf_counter_fork(struct task_struct *tsk); extern void perf_counter_task_migration(struct task_struct *task, int cpu); -#define MAX_STACK_DEPTH 255 - -struct perf_callchain_entry { - u16 nr; - u16 hv; - u16 kernel; - u16 user; - u64 ip[MAX_STACK_DEPTH]; -}; - extern struct perf_callchain_entry *perf_callchain(struct pt_regs *regs); extern int sysctl_perf_counter_paranoid; diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 986834623b43..e14e98676171 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -39,6 +39,8 @@ static int dump_trace = 0; #define cdprintf(x...) do { if (dump_trace) color_fprintf(stdout, color, x); } while (0) static int verbose; +#define eprintf(x...) do { if (verbose) fprintf(stderr, x); } while (0) + static int full_paths; static unsigned long page_size; @@ -47,14 +49,6 @@ static unsigned long mmap_window = 32; static char *parent_pattern = "^sys_|^do_page_fault"; static regex_t parent_regex; -struct ip_chain_event { - __u16 nr; - __u16 hv; - __u16 kernel; - __u16 user; - __u64 ips[]; -}; - struct ip_event { struct perf_event_header header; __u64 ip; @@ -131,15 +125,11 @@ static struct dso *dsos__findnew(const char *name) nr = dso__load(dso, NULL, verbose); if (nr < 0) { - if (verbose) - fprintf(stderr, "Failed to open: %s\n", name); + eprintf("Failed to open: %s\n", name); goto out_delete_dso; } - if (!nr && verbose) { - fprintf(stderr, - "No symbols found in: %s, maybe install a debug package?\n", - name); - } + if (!nr) + eprintf("No symbols found in: %s, maybe install a debug package?\n", name); dsos__add(dso); @@ -844,7 +834,7 @@ static struct symbol *call__match(struct symbol *sym) static int hist_entry__add(struct thread *thread, struct map *map, struct dso *dso, - struct symbol *sym, __u64 ip, struct ip_chain_event *chain, + struct symbol *sym, __u64 ip, struct perf_callchain_entry *chain, char level, __u64 count) { struct rb_node **p = &hist.rb_node; @@ -868,7 +858,7 @@ hist_entry__add(struct thread *thread, struct map *map, struct dso *dso, __u64 ip; for (i = 0; i < chain->kernel; i++) { - ip = chain->ips[nr + i]; + ip = chain->ip[nr + i]; dso = kernel_dso; sym = resolve_symbol(thread, NULL, &dso, &ip); entry.parent = call__match(sym); @@ -878,7 +868,7 @@ hist_entry__add(struct thread *thread, struct map *map, struct dso *dso, nr += i; for (i = 0; i < chain->user; i++) { - ip = chain->ips[nr + i]; + ip = chain->ip[nr + i]; sym = resolve_symbol(thread, NULL, NULL, &ip); entry.parent = call__match(sym); if (entry.parent) @@ -1080,6 +1070,30 @@ static unsigned long total = 0, total_fork = 0, total_unknown = 0; +static int validate_chain(struct perf_callchain_entry *chain, event_t *event) +{ + unsigned int chain_size; + + if (chain->nr > MAX_STACK_DEPTH) + return -1; + if (chain->hv > MAX_STACK_DEPTH) + return -1; + if (chain->kernel > MAX_STACK_DEPTH) + return -1; + if (chain->user > MAX_STACK_DEPTH) + return -1; + if (chain->hv + chain->kernel + chain->user != chain->nr) + return -1; + + chain_size = event->header.size; + chain_size -= (unsigned long)&event->ip.__more_data - (unsigned long)event; + + if (chain->nr*sizeof(__u64) > chain_size) + return -1; + + return 0; +} + static int process_overflow_event(event_t *event, unsigned long offset, unsigned long head) { @@ -1091,7 +1105,7 @@ process_overflow_event(event_t *event, unsigned long offset, unsigned long head) __u64 period = 1; struct map *map = NULL; void *more_data = event->ip.__more_data; - struct ip_chain_event *chain = NULL; + struct perf_callchain_entry *chain = NULL; if (event->header.type & PERF_SAMPLE_PERIOD) { period = *(__u64 *)more_data; @@ -1111,21 +1125,26 @@ process_overflow_event(event_t *event, unsigned long offset, unsigned long head) chain = (void *)more_data; - if (dump_trace) { - dprintf("... chain: u:%d, k:%d, nr:%d\n", - chain->user, - chain->kernel, - chain->nr); + dprintf("... chain: u:%d, k:%d, nr:%d\n", + chain->user, + chain->kernel, + chain->nr); + if (validate_chain(chain, event) < 0) { + eprintf("call-chain problem with event, skipping it.\n"); + return 0; + } + + if (dump_trace) { for (i = 0; i < chain->nr; i++) - dprintf("..... %2d: %016Lx\n", i, chain->ips[i]); + dprintf("..... %2d: %016Lx\n", i, chain->ip[i]); } } dprintf(" ... thread: %s:%d\n", thread->comm, thread->pid); if (thread == NULL) { - fprintf(stderr, "problem processing %d event, skipping it.\n", + eprintf("problem processing %d event, skipping it.\n", event->header.type); return -1; } @@ -1153,8 +1172,7 @@ process_overflow_event(event_t *event, unsigned long offset, unsigned long head) struct symbol *sym = resolve_symbol(thread, &map, &dso, &ip); if (hist_entry__add(thread, map, dso, sym, ip, chain, level, period)) { - fprintf(stderr, - "problem incrementing symbol count, skipping event\n"); + eprintf("problem incrementing symbol count, skipping event\n"); return -1; } } -- GitLab