From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753236AbbDHDxF (ORCPT ); Tue, 7 Apr 2015 23:53:05 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:29924 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751933AbbDHDxC (ORCPT ); Tue, 7 Apr 2015 23:53:02 -0400 From: Wang Nan To: CC: , , , , , Subject: [PATCH v4 2/2] perf: report/annotate: fix segfault problem. Date: Wed, 8 Apr 2015 03:52:18 +0000 Message-ID: <1428465138-124112-1-git-send-email-wangnan0@huawei.com> X-Mailer: git-send-email 1.8.3.4 In-Reply-To: <20150407151322.GK11983@kernel.org> References: <20150407151322.GK11983@kernel.org> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.107.197.200] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020202.5524A619.00F6,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: cfea15022020f4e31a1cf768a271bb5d Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org perf report and perf annotate are easy to trigger segfault if trace data contain kernel module information like this: # perf report -D -i ./perf.data ... 0 0 0x188 [0x50]: PERF_RECORD_MMAP -1/0: [0xffffffbff1018000(0xf068000) @ 0]: x [test_module] ... # perf report -i ./perf.data --objdump=/path/to/objdump --kallsyms=/path/to/kallsyms perf: Segmentation fault -------- backtrace -------- /path/to/perf[0x503478] /lib64/libc.so.6(+0x3545f)[0x7fb201f3745f] /path/to/perf[0x499b56] /path/to/perf(dso__load_kallsyms+0x13c)[0x49b56c] /path/to/perf(dso__load+0x72e)[0x49c21e] /path/to/perf(map__load+0x6e)[0x4ae9ee] /path/to/perf(thread__find_addr_map+0x24c)[0x47deec] /path/to/perf(perf_event__preprocess_sample+0x88)[0x47e238] /path/to/perf[0x43ad02] /path/to/perf[0x4b55bc] /path/to/perf(ordered_events__flush+0xca)[0x4b57ea] /path/to/perf[0x4b1a01] /path/to/perf(perf_session__process_events+0x3be)[0x4b428e] /path/to/perf(cmd_report+0xf11)[0x43bfc1] /path/to/perf[0x474702] /path/to/perf(main+0x5f5)[0x42de95] /lib64/libc.so.6(__libc_start_main+0xf4)[0x7fb201f23bd4] /path/to/perf[0x42dfc4] This is because __kmod_path__parse regard '[' leading name as kernel instead of kernel module. The DSO will then be passed to dso__load_kernel_sym() then dso__load_kcore() because of --kallsyms argument. The segfault is triggered because the kmap structure is not initialized. Although in --vmlinux case such segfault can be avoided, the symbols in the kernel module are unable to be retrived since the attribute of DSO is incorrect. This patch fixes __kmod_path__parse, make it regard names like '[test_module]' as kernel module. According to suggestion from Arnaldo Carvalho de Melo ( https://lkml.org/lkml/2015/4/6/90), appends cpumode argument to __kmod_path__parse() to ensure the passed path is a kernel mmap. kmod-path.c is also update to reflect the above changes. Signed-off-by: Wang Nan --- Different from v3: Don't warn if cpumode is unknown. --- tools/perf/tests/kmod-path.c | 43 +++++++++++++++++++++++++++++++---- tools/perf/util/dso.c | 54 +++++++++++++++++++++++++++++++++++++++----- tools/perf/util/dso.h | 12 ++++++---- tools/perf/util/header.c | 2 +- tools/perf/util/machine.c | 4 +++- 5 files changed, 98 insertions(+), 17 deletions(-) diff --git a/tools/perf/tests/kmod-path.c b/tools/perf/tests/kmod-path.c index e8d7cbb..5d57df6 100644 --- a/tools/perf/tests/kmod-path.c +++ b/tools/perf/tests/kmod-path.c @@ -4,14 +4,14 @@ #include "debug.h" static int test(const char *path, bool alloc_name, bool alloc_ext, - bool kmod, bool comp, const char *name, const char *ext) + bool kmod, bool comp, const char *name, const char *ext, int cpumode) { struct kmod_path m; memset(&m, 0x0, sizeof(m)); TEST_ASSERT_VAL("kmod_path__parse", - !__kmod_path__parse(&m, path, alloc_name, alloc_ext)); + !__kmod_path__parse(&m, path, cpumode, alloc_name, alloc_ext)); pr_debug("%s - alloc name %d, alloc ext %d, kmod %d, comp %d, name '%s', ext '%s'\n", path, alloc_name, alloc_ext, m.kmod, m.comp, m.name, m.ext); @@ -34,8 +34,14 @@ static int test(const char *path, bool alloc_name, bool alloc_ext, return 0; } -#define T(path, an, ae, k, c, n, e) \ - TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e)) + +#define _T(path, an, ae, k, c, n, e, m) \ + TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e, m)) + +#define T(path, an, ae, k, c, n, e) _T(path, an, ae, k, c, n, e, \ + PERF_RECORD_MISC_CPUMODE_UNKNOWN) +#define TU(path, an, ae, k, c, n, e) _T(path, an, ae, k, c, n, e, \ + PERF_RECORD_MISC_USER) int test__kmod_path__parse(void) { @@ -69,5 +75,34 @@ int test__kmod_path__parse(void) T("x.ko.gz", true , false , true, true, "[x]", NULL); T("x.ko.gz", false , false , true, true, NULL , NULL); + /* path alloc_name alloc_ext kmod comp name ext */ + T("[test_module]", true , true , true, false, "[test_module]" , NULL); + T("[test_module]", false , true , true, false, NULL , NULL); + T("[test_module]", true , false , true, false, "[test_module]" , NULL); + T("[test_module]", false , false , true, false, NULL , NULL); + + /* path alloc_name alloc_ext kmod comp name ext */ + T("[test.module]", true , true , true, false, "[test.module]" , NULL); + T("[test.module]", false , true , true, false, NULL , NULL); + T("[test.module]", true , false , true, false, "[test.module]" , NULL); + T("[test.module]", false , false , true, false, NULL , NULL); + + /* path alloc_name alloc_ext kmod comp name ext */ + TU("[vdso]", true , true , false, false, "[vdso]" , NULL); + TU("[vdso]", false , true , false, false, NULL , NULL); + TU("[vdso]", true , false , false, false, "[vdso]" , NULL); + TU("[vdso]", false , false , false, false, NULL , NULL); + + /* path alloc_name alloc_ext kmod comp name ext */ + TU("[vsyscall]", true , true , false, false, "[vsyscall]" , NULL); + TU("[vsyscall]", false , true , false, false, NULL , NULL); + TU("[vsyscall]", true , false , false, false, "[vsyscall]" , NULL); + TU("[vsyscall]", false , false , false, false, NULL , NULL); + + /* path alloc_name alloc_ext kmod comp name ext */ + T("[kernel.kallsyms]", true , true , false, false, "[kernel.kallsyms]" , NULL); + T("[kernel.kallsyms]", false , true , false, false, NULL , NULL); + T("[kernel.kallsyms]", true , false , false, false, "[kernel.kallsyms]" , NULL); + T("[kernel.kallsyms]", false , false , false, false, NULL , NULL); return 0; } diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index fc0ddd5..875f7c9 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -165,11 +165,11 @@ bool is_supported_compression(const char *ext) return false; } -bool is_kernel_module(const char *pathname) +bool is_kernel_module(const char *pathname, int cpumode) { struct kmod_path m; - if (kmod_path__parse(&m, pathname)) + if (kmod_path__parse(&m, pathname, cpumode)) return NULL; return m.kmod; @@ -210,21 +210,56 @@ bool dso__needs_decompress(struct dso *dso) * Returns 0 if there's no strdup error, -ENOMEM otherwise. */ int __kmod_path__parse(struct kmod_path *m, const char *path, - bool alloc_name, bool alloc_ext) + int cpumode, bool alloc_name, bool alloc_ext) { const char *name = strrchr(path, '/'); const char *ext = strrchr(path, '.'); + bool is_simple_name = false; + bool cpu_mode_kernel, cpu_mode_unknown = false, is_kernel = false; + + /* treat PERF_RECORD_MISC_CPUMODE_UNKNOWN as kernel. */ + switch (cpumode & PERF_RECORD_MISC_CPUMODE_MASK) { + case PERF_RECORD_MISC_USER: + case PERF_RECORD_MISC_HYPERVISOR: + case PERF_RECORD_MISC_GUEST_USER: + cpu_mode_kernel = false; + break; + case PERF_RECORD_MISC_CPUMODE_UNKNOWN: + cpu_mode_unknown = true; + /* fall through */ + default: + cpu_mode_kernel = true; + } memset(m, 0x0, sizeof(*m)); name = name ? name + 1 : path; + /* + * '.' is also a valid character. For example: [aaa.bbb] is a + * valid module name. '[' should have higher priority than + * '.ko' suffix. + * + * The kernel names are from machine__mmap_name. Such + * name should belong to kernel itself, not kernel module. + */ + if (name[0] == '[') { + is_simple_name = true; + if ((strncmp(name, "[kernel.kallsyms]", 17) == 0) || + (strncmp(name, "[guest.kernel.kallsyms", 22) == 0)) { + m->kmod = false; + is_kernel = true; + } else + m->kmod = cpu_mode_kernel; + } + /* No extension, just return name. */ - if (ext == NULL) { + if ((ext == NULL) || is_simple_name) { if (alloc_name) { m->name = strdup(name); - return m->name ? 0 : -ENOMEM; + if (!m->name) + return -ENOMEM; } - return 0; + goto out; } if (is_supported_compression(ext + 1)) { @@ -255,6 +290,13 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, return -ENOMEM; } } +out: + if (!cpu_mode_unknown) { + if ((m->kmod && !cpu_mode_kernel) || + (cpu_mode_kernel && !m->kmod && !is_kernel)) + pr_warning("Kmod (%s) and cpumode (%d) inconsistent\n", + path, cpumode); + } return 0; } diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index e0901b4..0079b2b 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -216,7 +216,7 @@ char dso__symtab_origin(const struct dso *dso); int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type, char *root_dir, char *filename, size_t size); bool is_supported_compression(const char *ext); -bool is_kernel_module(const char *pathname); +bool is_kernel_module(const char *pathname, int cpumode); bool decompress_to_file(const char *ext, const char *filename, int output_fd); bool dso__needs_decompress(struct dso *dso); @@ -228,11 +228,13 @@ struct kmod_path { }; int __kmod_path__parse(struct kmod_path *m, const char *path, - bool alloc_name, bool alloc_ext); + int cpumode, bool alloc_name, bool alloc_ext); -#define kmod_path__parse(__m, __p) __kmod_path__parse(__m, __p, false, false) -#define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, true , false) -#define kmod_path__parse_ext(__m, __p) __kmod_path__parse(__m, __p, false, true) +#define kmod_path__parse(__m, __p, __c) __kmod_path__parse(__m, __p, __c, false, false) +#define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, \ + PERF_RECORD_MISC_CPUMODE_UNKNOWN, true , false) +#define kmod_path__parse_ext(__m, __p) __kmod_path__parse(__m, __p, \ + PERF_RECORD_MISC_CPUMODE_UNKNOWN, false, true) /* * The dso__data_* external interface provides following functions: diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index fb43215..d106e12 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -1266,7 +1266,7 @@ static int __event_process_build_id(struct build_id_event *bev, dso__set_build_id(dso, &bev->build_id); - if (!is_kernel_module(filename)) + if (!is_kernel_module(filename, misc)) dso->kernel = dso_type; build_id__sprintf(dso->build_id, sizeof(dso->build_id), diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index e45c8f3..e084943 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -1109,7 +1109,9 @@ static int machine__process_kernel_mmap_event(struct machine *machine, struct dso *dso; list_for_each_entry(dso, &machine->kernel_dsos.head, node) { - if (is_kernel_module(dso->long_name)) + if (is_kernel_module(dso->long_name, + event->header.misc & + PERF_RECORD_MISC_CPUMODE_MASK)) continue; kernel = dso; -- 1.8.3.4