From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753886AbbDJDxW (ORCPT ); Thu, 9 Apr 2015 23:53:22 -0400 Received: from szxga01-in.huawei.com ([58.251.152.64]:10986 "EHLO szxga01-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753436AbbDJDxU (ORCPT ); Thu, 9 Apr 2015 23:53:20 -0400 From: Wang Nan To: CC: , , , , , , Subject: [PATCH v5 2/2] perf: report/annotate: fix segfault problem. Date: Fri, 10 Apr 2015 03:53:02 +0000 Message-ID: <1428637982-126924-1-git-send-email-wangnan0@huawei.com> X-Mailer: git-send-email 1.8.3.4 In-Reply-To: <20150409115231.GC1321@krava.brq.redhat.com> References: <20150409115231.GC1321@krava.brq.redhat.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [10.107.197.210] X-CFilter-Loop: Reflected 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. If perf.data contain build information and the buildid of such modules can be found, the DSO of it will be treated as kernel, not kernel module. It 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 to treat names like '[test_module]' as kernel modules. kmod-path.c is also update to reflect the above changes. Signed-off-by: Wang Nan --- Different from v4: checks cpumode in is_kernel_module(), makes code simpler. Appends tests of is_kernel_module(). --- tools/perf/tests/kmod-path.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/dso.c | 42 +++++++++++++++++++++++--- tools/perf/util/dso.h | 2 +- tools/perf/util/header.c | 8 ++--- tools/perf/util/machine.c | 16 +++++++++- 5 files changed, 130 insertions(+), 10 deletions(-) diff --git a/tools/perf/tests/kmod-path.c b/tools/perf/tests/kmod-path.c index e8d7cbb..08c433b 100644 --- a/tools/perf/tests/kmod-path.c +++ b/tools/perf/tests/kmod-path.c @@ -34,9 +34,21 @@ static int test(const char *path, bool alloc_name, bool alloc_ext, return 0; } +static int test_is_kernel_module(const char *path, int cpumode, bool expect) +{ + TEST_ASSERT_VAL("is_kernel_module", + (!!is_kernel_module(path, cpumode)) == (!!expect)); + pr_debug("%s (cpumode: %d) - is_kernel_module: %s\n", + path, cpumode, expect ? "true" : "false"); + return 0; +} + #define T(path, an, ae, k, c, n, e) \ TEST_ASSERT_VAL("failed", !test(path, an, ae, k, c, n, e)) +#define M(path, c, e) \ + TEST_ASSERT_VAL("failed", !test_is_kernel_module(path, c, e)) + int test__kmod_path__parse(void) { /* path alloc_name alloc_ext kmod comp name ext */ @@ -44,30 +56,90 @@ int test__kmod_path__parse(void) T("/xxxx/xxxx/x-x.ko", false , true , true, false, NULL , NULL); T("/xxxx/xxxx/x-x.ko", true , false , true, false, "[x_x]", NULL); T("/xxxx/xxxx/x-x.ko", false , false , true, false, NULL , NULL); + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_KERNEL, true); + M("/xxxx/xxxx/x-x.ko", PERF_RECORD_MISC_USER, false); /* path alloc_name alloc_ext kmod comp name ext */ T("/xxxx/xxxx/x.ko.gz", true , true , true, true, "[x]", "gz"); T("/xxxx/xxxx/x.ko.gz", false , true , true, true, NULL , "gz"); T("/xxxx/xxxx/x.ko.gz", true , false , true, true, "[x]", NULL); T("/xxxx/xxxx/x.ko.gz", false , false , true, true, NULL , NULL); + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_KERNEL, true); + M("/xxxx/xxxx/x.ko.gz", PERF_RECORD_MISC_USER, false); /* path alloc_name alloc_ext kmod comp name ext */ T("/xxxx/xxxx/x.gz", true , true , false, true, "x.gz" ,"gz"); T("/xxxx/xxxx/x.gz", false , true , false, true, NULL ,"gz"); T("/xxxx/xxxx/x.gz", true , false , false, true, "x.gz" , NULL); T("/xxxx/xxxx/x.gz", false , false , false, true, NULL , NULL); + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_KERNEL, false); + M("/xxxx/xxxx/x.gz", PERF_RECORD_MISC_USER, false); /* path alloc_name alloc_ext kmod comp name ext */ T("x.gz", true , true , false, true, "x.gz", "gz"); T("x.gz", false , true , false, true, NULL , "gz"); T("x.gz", true , false , false, true, "x.gz", NULL); T("x.gz", false , false , false, true, NULL , NULL); + M("x.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); + M("x.gz", PERF_RECORD_MISC_KERNEL, false); + M("x.gz", PERF_RECORD_MISC_USER, false); /* path alloc_name alloc_ext kmod comp name ext */ T("x.ko.gz", true , true , true, true, "[x]", "gz"); T("x.ko.gz", false , true , true, true, NULL , "gz"); T("x.ko.gz", true , false , true, true, "[x]", NULL); T("x.ko.gz", false , false , true, true, NULL , NULL); + M("x.ko.gz", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); + M("x.ko.gz", PERF_RECORD_MISC_KERNEL, true); + M("x.ko.gz", PERF_RECORD_MISC_USER, false); + + /* 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); + M("[test_module]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); + M("[test_module]", PERF_RECORD_MISC_KERNEL, true); + M("[test_module]", PERF_RECORD_MISC_USER, false); + + /* 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); + M("[test.module]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, true); + M("[test.module]", PERF_RECORD_MISC_KERNEL, true); + M("[test.module]", PERF_RECORD_MISC_USER, false); + + /* path alloc_name alloc_ext kmod comp name ext */ + T("[vdso]", true , true , false, false, "[vdso]", NULL); + T("[vdso]", false , true , false, false, NULL , NULL); + T("[vdso]", true , false , false, false, "[vdso]", NULL); + T("[vdso]", false , false , false, false, NULL , NULL); + M("[vdso]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); + M("[vdso]", PERF_RECORD_MISC_KERNEL, false); + M("[vdso]", PERF_RECORD_MISC_USER, false); + + /* path alloc_name alloc_ext kmod comp name ext */ + T("[vsyscall]", true , true , false, false, "[vsyscall]", NULL); + T("[vsyscall]", false , true , false, false, NULL , NULL); + T("[vsyscall]", true , false , false, false, "[vsyscall]", NULL); + T("[vsyscall]", false , false , false, false, NULL , NULL); + M("[vsyscall]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); + M("[vsyscall]", PERF_RECORD_MISC_KERNEL, false); + M("[vsyscall]", PERF_RECORD_MISC_USER, false); + + /* 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); + M("[kernel.kallsyms]", PERF_RECORD_MISC_CPUMODE_UNKNOWN, false); + M("[kernel.kallsyms]", PERF_RECORD_MISC_KERNEL, false); + M("[kernel.kallsyms]", PERF_RECORD_MISC_USER, false); return 0; } diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index fc0ddd5..e9d4ae4 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -165,13 +165,25 @@ 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)) - return NULL; + /* caller should pass a masked cpumode. Mask again for safety. */ + switch (cpumode & PERF_RECORD_MISC_CPUMODE_MASK) { + case PERF_RECORD_MISC_USER: + case PERF_RECORD_MISC_HYPERVISOR: + case PERF_RECORD_MISC_GUEST_USER: + return false; + /* Regard PERF_RECORD_MISC_CPUMODE_UNKNOWN as kernel */ + default: + if (kmod_path__parse(&m, pathname)) { + pr_err("Failed to check whether %s is a kernel module or not. Assume it is.", + pathname); + return true; + } + } return m.kmod; } @@ -214,12 +226,34 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, { const char *name = strrchr(path, '/'); const char *ext = strrchr(path, '.'); + bool is_simple_name = false; memset(m, 0x0, sizeof(*m)); name = name ? name + 1 : path; + /* + * '.' is also a valid character for module name. 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) || + (strncmp(name, "[vdso]", 6) == 0) || + (strncmp(name, "[vsyscall]", 10) == 0)) { + m->kmod = false; + + } else + m->kmod = true; + } + + /* 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; diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index e0901b4..cc3797c 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); diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index fb43215..8c76a23 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -1232,7 +1232,7 @@ static int __event_process_build_id(struct build_id_event *bev, int err = -1; struct dsos *dsos; struct machine *machine; - u16 misc; + u16 cpumode; struct dso *dso; enum dso_kernel_type dso_type; @@ -1240,9 +1240,9 @@ static int __event_process_build_id(struct build_id_event *bev, if (!machine) goto out; - misc = bev->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; + cpumode = bev->header.misc & PERF_RECORD_MISC_CPUMODE_MASK; - switch (misc) { + switch (cpumode) { case PERF_RECORD_MISC_KERNEL: dso_type = DSO_TYPE_KERNEL; dsos = &machine->kernel_dsos; @@ -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, cpumode)) 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 e335330..3769009 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -1109,7 +1109,21 @@ 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)) + /* + * cpumode passed to is_kernel_module is not the + * cpumode of *this* event. If we insist on passing + * correct cpumode to is_kernel_module, we should record + * the cpumode when we adding this dso to the linked list. + * + * However we don't really need passing correct cpumode. + * We know the correct cpumode must be kernel mode + * (if not, we should not link it onto kernel_dsos list). + * + * Therefore, we pass PERF_RECORD_MISC_CPUMODE_UNKNOWN. + * is_kernel_module() treat it as a kernel cpumode. + */ + if (is_kernel_module(dso->long_name, + PERF_RECORD_MISC_CPUMODE_UNKNOWN)) continue; kernel = dso; -- 1.8.3.4