[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1428394966-131044-3-git-send-email-wangnan0@huawei.com>
Date: Tue, 7 Apr 2015 08:22:46 +0000
From: Wang Nan <wangnan0@...wei.com>
To: <acme@...nel.org>, <jolsa@...nel.org>, <namhyung@...nel.org>,
<mingo@...hat.com>
CC: <lizefan@...wei.com>, <pi3orama@....com>,
<linux-kernel@...r.kernel.org>
Subject: [PATCH v3 2/2] perf: report/annotate: fix segfault problem.
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 <wangnan0@...wei.com>
---
tools/perf/tests/kmod-path.c | 43 ++++++++++++++++++++++++++++++++++----
tools/perf/util/dso.c | 49 ++++++++++++++++++++++++++++++++++++++------
tools/perf/util/dso.h | 12 ++++++-----
tools/perf/util/header.c | 2 +-
tools/perf/util/machine.c | 4 +++-
5 files changed, 93 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..fb1ed7d 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,53 @@ 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, 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;
+ 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 +287,11 @@ int __kmod_path__parse(struct kmod_path *m, const char *path,
return -ENOMEM;
}
}
+out:
+ 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 9c7feec..c9e7e57 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1112,7 +1112,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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists