lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ