mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Wang Nan <wangnan0@huawei.com>
To: <jolsa@redhat.com>, <namhyung@kernel.org>, <jolsa@kernel.org>,
	<acme@redhat.com>, <mingo@kernel.org>
Cc: <linux-kernel@vger.kernel.org>, <pi3orama@163.com>
Subject: [PATCH] perf: kmaps: enforce usage of kmaps to protect futher bugs.
Date: Fri, 3 Apr 2015 08:47:57 +0000	[thread overview]
Message-ID: <1428050877-54093-1-git-send-email-wangnan0@huawei.com> (raw)
In-Reply-To: <20150403064822.GB29383@gmail.com>

This patch add checkings on every place where uses map__kmap and get
kmaps from struct kmap. Error messages are added at map__kmap to warn
invalud accessing of kmap (for the case of !map->dso->kernel, kmap(map)
is not exists at all). Also, introduces map__kmaps() to warn
uninitialized kmaps.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
---
 tools/perf/util/machine.c     |  5 ++++-
 tools/perf/util/map.h         | 15 +++++++++++++++
 tools/perf/util/probe-event.c |  2 ++
 tools/perf/util/session.c     |  3 +++
 tools/perf/util/symbol-elf.c  | 16 +++++++++++-----
 tools/perf/util/symbol.c      | 34 ++++++++++++++++++++++++++++------
 6 files changed, 63 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index e335330..9c7feec 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -679,6 +679,9 @@ int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel)
 			machine->vmlinux_maps[type]->unmap_ip =
 				identity__map_ip;
 		kmap = map__kmap(machine->vmlinux_maps[type]);
+		if (!kmap)
+			return -1;
+
 		kmap->kmaps = &machine->kmaps;
 		map_groups__insert(&machine->kmaps,
 				   machine->vmlinux_maps[type]);
@@ -700,7 +703,7 @@ void machine__destroy_kernel_maps(struct machine *machine)
 		kmap = map__kmap(machine->vmlinux_maps[type]);
 		map_groups__remove(&machine->kmaps,
 				   machine->vmlinux_maps[type]);
-		if (kmap->ref_reloc_sym) {
+		if (kmap && kmap->ref_reloc_sym) {
 			/*
 			 * ref_reloc_sym is shared among all maps, so free just
 			 * on one of them.
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 0e42438..2ba83c8 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -78,9 +78,24 @@ void map_groups__put(struct map_groups *mg);
 
 static inline struct kmap *map__kmap(struct map *map)
 {
+	if (!map->dso || !map->dso->kernel) {
+		pr_err("Internal error: map__kmap with a non-kernel map\n");
+		return NULL;
+	}
 	return (struct kmap *)(map + 1);
 }
 
+static inline struct map_groups *kmaps map__kmaps(struct map *map)
+{
+	struct kmap *kmap = map__kmap(map);
+
+	if (!kmap || !kmap->kmaps) {
+		pr_err("Internal error: map__kmaps with a non-kernel map\n");
+		return NULL;
+	}
+	return kmap->kmaps;
+}
+
 static inline u64 map__map_ip(struct map *map, u64 ip)
 {
 	return ip - map->start + map->pgoff;
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 8feac07..4fd49f0 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -135,6 +135,8 @@ static struct ref_reloc_sym *kernel_get_ref_reloc_sym(void)
 		return NULL;
 
 	kmap = map__kmap(host_machine->vmlinux_maps[MAP__FUNCTION]);
+	if (!kmap)
+		return NULL;
 	return kmap->ref_reloc_sym;
 }
 
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index adf0740..7caedb4 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1456,6 +1456,9 @@ int maps__set_kallsyms_ref_reloc_sym(struct map **maps,
 
 	for (i = 0; i < MAP__NR_TYPES; ++i) {
 		struct kmap *kmap = map__kmap(maps[i]);
+
+		if (!kmap)
+			continue;
 		kmap->ref_reloc_sym = ref;
 	}
 
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 476268c..a7ab606 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -776,6 +776,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
 		  symbol_filter_t filter, int kmodule)
 {
 	struct kmap *kmap = dso->kernel ? map__kmap(map) : NULL;
+	struct map_groups *kmaps = kmap ? map__kmaps(map) : NULL;
 	struct map *curr_map = map;
 	struct dso *curr_dso = dso;
 	Elf_Data *symstrs, *secstrs;
@@ -791,6 +792,9 @@ int dso__load_sym(struct dso *dso, struct map *map,
 	int nr = 0;
 	bool remap_kernel = false, adjust_kernel_syms = false;
 
+	if (kmap && !kmaps)
+		return -1;
+
 	dso->symtab_type = syms_ss->type;
 	dso->is_64_bit = syms_ss->is_64_bit;
 	dso->rel = syms_ss->ehdr.e_type == ET_REL;
@@ -958,8 +962,10 @@ int dso__load_sym(struct dso *dso, struct map *map,
 					map->map_ip = map__map_ip;
 					map->unmap_ip = map__unmap_ip;
 					/* Ensure maps are correctly ordered */
-					map_groups__remove(kmap->kmaps, map);
-					map_groups__insert(kmap->kmaps, map);
+					if (kmaps) {
+						map_groups__remove(kmaps, map);
+						map_groups__insert(kmaps, map);
+					}
 				}
 
 				/*
@@ -983,7 +989,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
 			snprintf(dso_name, sizeof(dso_name),
 				 "%s%s", dso->short_name, section_name);
 
-			curr_map = map_groups__find_by_name(kmap->kmaps, map->type, dso_name);
+			curr_map = map_groups__find_by_name(kmaps, map->type, dso_name);
 			if (curr_map == NULL) {
 				u64 start = sym.st_value;
 
@@ -1013,7 +1019,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
 					curr_map->unmap_ip = identity__map_ip;
 				}
 				curr_dso->symtab_type = dso->symtab_type;
-				map_groups__insert(kmap->kmaps, curr_map);
+				map_groups__insert(kmaps, curr_map);
 				/*
 				 * The new DSO should go to the kernel DSOS
 				 */
@@ -1075,7 +1081,7 @@ new_symbol:
 			 * We need to fixup this here too because we create new
 			 * maps here, for things like vsyscall sections.
 			 */
-			__map_groups__fixup_end(kmap->kmaps, map->type);
+			__map_groups__fixup_end(kmaps, map->type);
 		}
 	}
 	err = nr;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index fddeb90..201f6c4c 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -630,13 +630,16 @@ static int dso__load_all_kallsyms(struct dso *dso, const char *filename,
 static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 					 symbol_filter_t filter)
 {
-	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	struct map_groups *kmaps = map__kmaps(map);
 	struct map *curr_map;
 	struct symbol *pos;
 	int count = 0, moved = 0;
 	struct rb_root *root = &dso->symbols[map->type];
 	struct rb_node *next = rb_first(root);
 
+	if (!kmaps)
+		return -1;
+
 	while (next) {
 		char *module;
 
@@ -682,8 +685,8 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
 static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
 			       symbol_filter_t filter)
 {
-	struct map_groups *kmaps = map__kmap(map)->kmaps;
-	struct machine *machine = kmaps->machine;
+	struct map_groups *kmaps = map__kmaps(map);
+	struct machine *machine;
 	struct map *curr_map = map;
 	struct symbol *pos;
 	int count = 0, moved = 0;
@@ -691,6 +694,11 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
 	struct rb_node *next = rb_first(root);
 	int kernel_range = 0;
 
+	if (!kmaps)
+		return -1;
+
+	machine = kmaps->machine;
+
 	while (next) {
 		char *module;
 
@@ -1025,9 +1033,12 @@ static bool filename_from_kallsyms_filename(char *filename,
 static int validate_kcore_modules(const char *kallsyms_filename,
 				  struct map *map)
 {
-	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	struct map_groups *kmaps = map__kmaps(map);
 	char modules_filename[PATH_MAX];
 
+	if (!kmaps)
+		return -EINVAL;
+
 	if (!filename_from_kallsyms_filename(modules_filename, "modules",
 					     kallsyms_filename))
 		return -EINVAL;
@@ -1043,6 +1054,9 @@ static int validate_kcore_addresses(const char *kallsyms_filename,
 {
 	struct kmap *kmap = map__kmap(map);
 
+	if (!kmap)
+		return -EINVAL;
+
 	if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) {
 		u64 start;
 
@@ -1081,8 +1095,8 @@ static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
 static int dso__load_kcore(struct dso *dso, struct map *map,
 			   const char *kallsyms_filename)
 {
-	struct map_groups *kmaps = map__kmap(map)->kmaps;
-	struct machine *machine = kmaps->machine;
+	struct map_groups *kmaps = map__kmaps(map);
+	struct machine *machine;
 	struct kcore_mapfn_data md;
 	struct map *old_map, *new_map, *replacement_map = NULL;
 	bool is_64_bit;
@@ -1090,6 +1104,11 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	char kcore_filename[PATH_MAX];
 	struct symbol *sym;
 
+	if (!kmaps)
+		return -EINVAL;
+
+	machine = kmaps->machine;
+
 	/* This function requires that the map is the kernel map */
 	if (map != machine->vmlinux_maps[map->type])
 		return -EINVAL;
@@ -1202,6 +1221,9 @@ static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
 	struct kmap *kmap = map__kmap(map);
 	u64 addr;
 
+	if (!kmap)
+		return -1;
+
 	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
 		return 0;
 
-- 
1.8.3.4


  reply	other threads:[~2015-04-03  8:48 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-03  5:56 [PATCH v2] perf: report/annotate: fix segfault problem Wang Nan
2015-04-03  6:48 ` Ingo Molnar
2015-04-03  8:47   ` Wang Nan [this message]
2015-04-03  8:49     ` [PATCH] perf: kmaps: enforce usage of kmaps to protect futher bugs Ingo Molnar
2015-04-03 11:11     ` Jiri Olsa
2015-04-07 10:42       ` Adrian Hunter
2015-04-08 10:50         ` [PATCH v4] " Wang Nan
2015-04-03  9:07 ` [PATCH v2] perf: report/annotate: fix segfault problem Jiri Olsa
2015-04-03 10:57 ` Jiri Olsa
2015-04-06 12:52   ` Arnaldo Carvalho de Melo
2015-04-07  8:22     ` [PATCH v3 0/2] " Wang Nan
2015-04-07  8:22       ` [PATCH v3 1/2] perf: kmaps: enforce usage of kmaps to protect futher bugs Wang Nan
2015-04-08 15:10         ` [tip:perf/core] perf kmaps: Check kmaps to make code more robust tip-bot for Wang Nan
2015-04-07  8:22       ` [PATCH v3 2/2] perf: report/annotate: fix segfault problem Wang Nan
2015-04-07 15:13         ` Arnaldo Carvalho de Melo
2015-04-08  3:49           ` Wang Nan
2015-04-08  3:52           ` [PATCH v4 " Wang Nan
2015-04-08 13:59             ` Jiri Olsa
2015-04-09  7:05               ` Wang Nan
2015-04-09 11:40                 ` Jiri Olsa
2015-04-09 11:52             ` Jiri Olsa
2015-04-10  1:57               ` Wang Nan
2015-04-10  2:49               ` Wang Nan
2015-04-10  3:53               ` [PATCH v5 " Wang Nan
2015-04-15  1:27                 ` Wang Nan
2015-04-20  1:18                   ` Wang Nan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1428050877-54093-1-git-send-email-wangnan0@huawei.com \
    --to=wangnan0@huawei.com \
    --cc=acme@redhat.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=pi3orama@163.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®