Re: [PATCH V6 1/8] perf tools: validate kcore module addresses

Re: [PATCH V6 1/8] perf tools: validate kcore module addresses

[Arnaldo Carvalho de Melo]

Em Wed, Oct 09, 2013 at 03:01:11PM +0300, Adrian Hunter escreveu:
> -static bool kcore_filename_from_kallsyms_filename(char *kcore_filename,
> -						  const char *kallsyms_filename)
> +static bool filename_from_kallsyms_filename(char *filename,
> +					    const char *basename,
> +					    const char *kallsyms_filename)

In Fedora12:

  CC       /tmp/build/perf/util/symbol.o
cc1: warnings being treated as errors
util/symbol.c: In function ‘filename_from_kallsyms_filename’:
util/symbol.c:937: error: declaration of ‘__xpg_basename’ shadows a
global declaration
/usr/include/libgen.h:35: error: shadowed declaration is here
make[1]: *** [/tmp/build/perf/util/symbol.o] Error 1
make: *** [install] Error 2
make: Leaving directory `/home/acme/git/linux/tools/perf'
[acme@fedora12 linux]$ 

Fixing it up renaming it to base_name,


Oops, one more:

  CC       /tmp/build/perf/util/symbol-elf.o
cc1: warnings being treated as errors
util/symbol-elf.c: In function ‘kcore__add_phdr’:
util/symbol-elf.c:1167: error: declaration of ‘index’ shadows a global
declaration
/usr/include/string.h:487: error: shadowed declaration is here
util/symbol-elf.c: In function ‘kcore_extract__create’:
util/symbol-elf.c:1202: error: declaration of ‘index’ shadows a global
declaration
/usr/include/string.h:487: error: shadowed declaration is here
make[1]: *** [/tmp/build/perf/util/symbol-elf.o] Error 1
make: *** [install] Error 2
make: Leaving directory `/home/acme/git/linux/tools/perf'
[acme@fedora12 linux]$

Will switch to 'idx'.

- Arnaldo

----- End forwarded message -----

Re: [PATCH V6 1/8] perf tools: validate kcore module addresses

[Arnaldo Carvalho de Melo]

One more, looking into it...


  CC       /tmp/build/perf/util/symbol-elf.o
cc1: warnings being treated as errors
util/symbol-elf.c: In function ‘copy_bytes’:
util/symbol-elf.c:1021: error: not protecting local variables: variable
length buffer
make[1]: *** [/tmp/build/perf/util/symbol-elf.o] Error 1
make: *** [install] Error 2
make: Leaving directory `/home/acme/git/linux/tools/perf'
[acme@fedora12 linux]$ 

[PATCH V6 0/8] perf tools: kcore improvements

[Adrian Hunter]

Hi

Here are some improvements for using kcore (version 6).  There are 3
improvements:

	- validate that kcore matches the perf.data modules
	- workaround objdump difficulties with kcore
	- add kcore to the build-id cache

Changes in V6:
	perf tools: make a separate function to parse /proc/modules
		Dropped because it has been applied
	perf tools: workaround objdump difficulties with kcore
		Renamed some functions and reordered parameters
	perf buildid-cache: add ability to add kcore to the cache
		Renamed some functions and reordered parameters

Changes in V5:
	perf tools: make a separate function to parse /proc/modules
		Use scnprintf not snprintf
	perf tools: validate kcore module addresses
		Fix check for mi->name not allocated
	perf buildid-cache: add ability to add kcore to the cache
		Use scnprintf not snprintf
	perf tools: add ability to find kcore in build-id cache
		Use scnprintf not snprintf

Changes in V4:
	perf tools: fix path unpopulated in machine__create_modules()
		Dropped because it has been applied
	perf buildid-cache: add ability to add kcore to the cache
		Tweaked Documentation/perf-buildid-cache.txt
	perf tools: add ability to find kcore in build-id cache
		Changed to check read access to /proc/kcore before
		skipping the buildid cache

Changes in V3:
	perf tools: workaround objdump difficulties with kcore
		change strncpy to strlcpy
	perf buildid-cache: add ability to add kcore to the cache
		change strncpy to strlcpy
	perf tools: add ability to find kcore in build-id cache
		change strncpy to strlcpy
Changes in V2:
	perf tools: fix buildid cache handling of kallsyms with kcore
		Dropped because it has been applied
	perf tools: fix path unpopulated in machine__create_modules()
		Use 'modules' pointer


Adrian Hunter (8):
      perf tools: validate kcore module addresses
      perf tools: workaround objdump difficulties with kcore
      perf tools: add map__find_other_map_symbol()
      perf tools: fix annotate_browser__callq()
      perf tools: find kcore symbols on other maps
      perf tools: add copyfile_mode()
      perf buildid-cache: add ability to add kcore to the cache
      perf tools: add ability to find kcore in build-id cache

 tools/perf/Documentation/perf-buildid-cache.txt |  13 +
 tools/perf/builtin-buildid-cache.c              | 148 +++++-
 tools/perf/ui/browsers/annotate.c               |  10 +-
 tools/perf/util/annotate.c                      |  36 +-
 tools/perf/util/map.c                           |  27 ++
 tools/perf/util/map.h                           |   2 +
 tools/perf/util/symbol-elf.c                    | 579 ++++++++++++++++++++++++
 tools/perf/util/symbol-minimal.c                |  15 +
 tools/perf/util/symbol.c                        | 384 +++++++++++++---
 tools/perf/util/symbol.h                        |  17 +
 tools/perf/util/util.c                          |  18 +-
 tools/perf/util/util.h                          |   1 +
 12 files changed, 1165 insertions(+), 85 deletions(-)


Regards
Adrian

[PATCH V6 1/8] perf tools: validate kcore module addresses

[Adrian Hunter]

Before using kcore we need to check that modules are
in memory at the same addresses that they were when
data was recorded.

This is done because, while we could remap symbols
to different addresses, the object code linkages
would still be different which would provide an
erroneous view of the object code.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 196 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 175 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5fd9513..2a2c581 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -798,51 +798,201 @@ bool symbol__restricted_filename(const char *filename,
 	return restricted;
 }
 
-struct kcore_mapfn_data {
-	struct dso *dso;
-	enum map_type type;
-	struct list_head maps;
+struct module_info {
+	struct rb_node rb_node;
+	char *name;
+	u64 start;
 };
 
-static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
+static void add_module(struct module_info *mi, struct rb_root *modules)
 {
-	struct kcore_mapfn_data *md = data;
-	struct map *map;
+	struct rb_node **p = &modules->rb_node;
+	struct rb_node *parent = NULL;
+	struct module_info *m;
 
-	map = map__new2(start, md->dso, md->type);
-	if (map == NULL)
+	while (*p != NULL) {
+		parent = *p;
+		m = rb_entry(parent, struct module_info, rb_node);
+		if (strcmp(mi->name, m->name) < 0)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+	rb_link_node(&mi->rb_node, parent, p);
+	rb_insert_color(&mi->rb_node, modules);
+}
+
+static void delete_modules(struct rb_root *modules)
+{
+	struct module_info *mi;
+	struct rb_node *next = rb_first(modules);
+
+	while (next) {
+		mi = rb_entry(next, struct module_info, rb_node);
+		next = rb_next(&mi->rb_node);
+		rb_erase(&mi->rb_node, modules);
+		free(mi->name);
+		free(mi);
+	}
+}
+
+static struct module_info *find_module(const char *name,
+				       struct rb_root *modules)
+{
+	struct rb_node *n = modules->rb_node;
+
+	while (n) {
+		struct module_info *m;
+		int cmp;
+
+		m = rb_entry(n, struct module_info, rb_node);
+		cmp = strcmp(name, m->name);
+		if (cmp < 0)
+			n = n->rb_left;
+		else if (cmp > 0)
+			n = n->rb_right;
+		else
+			return m;
+	}
+
+	return NULL;
+}
+
+static int __read_proc_modules(void *arg, const char *name, u64 start)
+{
+	struct rb_root *modules = arg;
+	struct module_info *mi;
+
+	mi = zalloc(sizeof(struct module_info));
+	if (!mi)
 		return -ENOMEM;
 
-	map->end = map->start + len;
-	map->pgoff = pgoff;
+	mi->name = strdup(name);
+	mi->start = start;
 
-	list_add(&map->node, &md->maps);
+	if (!mi->name) {
+		free(mi);
+		return -ENOMEM;
+	}
+
+	add_module(mi, modules);
+
+	return 0;
+}
+
+static int read_proc_modules(const char *filename, struct rb_root *modules)
+{
+	if (symbol__restricted_filename(filename, "/proc/modules"))
+		return -1;
+
+	if (modules__parse(filename, modules, __read_proc_modules)) {
+		delete_modules(modules);
+		return -1;
+	}
 
 	return 0;
 }
 
+static int do_validate_kcore_modules(const char *filename, struct map *map,
+				  struct map_groups *kmaps)
+{
+	struct rb_root modules = RB_ROOT;
+	struct map *old_map;
+	int err;
+
+	err = read_proc_modules(filename, &modules);
+	if (err)
+		return err;
+
+	old_map = map_groups__first(kmaps, map->type);
+	while (old_map) {
+		struct map *next = map_groups__next(old_map);
+		struct module_info *mi;
+
+		if (old_map == map || old_map->start == map->start) {
+			/* The kernel map */
+			old_map = next;
+			continue;
+		}
+
+		/* Module must be in memory at the same address */
+		mi = find_module(old_map->dso->short_name, &modules);
+		if (!mi || mi->start != old_map->start) {
+			err = -EINVAL;
+			goto out;
+		}
+
+		old_map = next;
+	}
+out:
+	delete_modules(&modules);
+	return err;
+}
+
 /*
- * If kallsyms is referenced by name then we look for kcore in the same
+ * If kallsyms is referenced by name then we look for filename in the same
  * directory.
  */
-static bool kcore_filename_from_kallsyms_filename(char *kcore_filename,
-						  const char *kallsyms_filename)
+static bool filename_from_kallsyms_filename(char *filename,
+					    const char *basename,
+					    const char *kallsyms_filename)
 {
 	char *name;
 
-	strcpy(kcore_filename, kallsyms_filename);
-	name = strrchr(kcore_filename, '/');
+	strcpy(filename, kallsyms_filename);
+	name = strrchr(filename, '/');
 	if (!name)
 		return false;
 
-	if (!strcmp(name, "/kallsyms")) {
-		strcpy(name, "/kcore");
+	name += 1;
+
+	if (!strcmp(name, "kallsyms")) {
+		strcpy(name, basename);
 		return true;
 	}
 
 	return false;
 }
 
+static int validate_kcore_modules(const char *kallsyms_filename,
+				  struct map *map)
+{
+	struct map_groups *kmaps = map__kmap(map)->kmaps;
+	char modules_filename[PATH_MAX];
+
+	if (!filename_from_kallsyms_filename(modules_filename, "modules",
+					     kallsyms_filename))
+		return -EINVAL;
+
+	if (do_validate_kcore_modules(modules_filename, map, kmaps))
+		return -EINVAL;
+
+	return 0;
+}
+
+struct kcore_mapfn_data {
+	struct dso *dso;
+	enum map_type type;
+	struct list_head maps;
+};
+
+static int kcore_mapfn(u64 start, u64 len, u64 pgoff, void *data)
+{
+	struct kcore_mapfn_data *md = data;
+	struct map *map;
+
+	map = map__new2(start, md->dso, md->type);
+	if (map == NULL)
+		return -ENOMEM;
+
+	map->end = map->start + len;
+	map->pgoff = pgoff;
+
+	list_add(&map->node, &md->maps);
+
+	return 0;
+}
+
 static int dso__load_kcore(struct dso *dso, struct map *map,
 			   const char *kallsyms_filename)
 {
@@ -859,8 +1009,12 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 	if (map != machine->vmlinux_maps[map->type])
 		return -EINVAL;
 
-	if (!kcore_filename_from_kallsyms_filename(kcore_filename,
-						   kallsyms_filename))
+	if (!filename_from_kallsyms_filename(kcore_filename, "kcore",
+					     kallsyms_filename))
+		return -EINVAL;
+
+	/* All modules must be present at their original addresses */
+	if (validate_kcore_modules(kallsyms_filename, map))
 		return -EINVAL;
 
 	md.dso = dso;
-- 
1.7.11.7