linux-kernel - Re: [PATCH/RFC 6/9] perf symbols: Use already loaded module dso when loading kcore

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]

Date:   Fri, 23 Jun 2017 10:55:47 -0300
From:   Arnaldo Carvalho de Melo <acme@...nel.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Peter Zijlstra <a.p.zijlstra@...llo.nl>,
        Jiri Olsa <jolsa@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>, kernel-team@....com,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Andi Kleen <andi@...stfloor.org>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Wang Nan <wangnan0@...wei.com>
Subject: Re: [PATCH/RFC 6/9] perf symbols: Use already loaded module dso when
 loading kcore

Em Fri, Jun 23, 2017 at 02:48:24PM +0900, Namhyung Kim escreveu:
> Even every module has loaded onto same addresses, some modules can be
> changed and reloaded.

Can you rephrase the above statement?

You mean even if a module is reloaded at the same address it can have a
different symtab and thus misresolve symbols if we get a sample for one
module version and try to resolve symbols using a symtab for another
module version?

> In that case it needs to access to the old module
> in the build-id cache.

Right, we need to make sure that during a 'perf record' session a módule
isn't reloaded, because at 'perf record' exit we will record that module
build-id in a perf.data header and grab a copy (or a hardlink, if
possible) into the ~/.debug build-id cache, then, at 'perf report' time
we will read that build-id perf.data table and lookup in the build-id
cache to get the matching symtab (and full elf .ko file to do things
like annotation).

- Arnaldo
 
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Wang Nan <wangnan0@...wei.com>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/util/symbol.c | 45 +++++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
> index ce79a51f25bf..fe46eb782297 100644
> --- a/tools/perf/util/symbol.c
> +++ b/tools/perf/util/symbol.c
> @@ -1155,6 +1155,7 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
>  	int err, fd;
>  	char kcore_filename[PATH_MAX];
>  	struct symbol *sym;
> +	struct map_groups old_mg;
>  
>  	if (!kmaps)
>  		return -EINVAL;
> @@ -1196,15 +1197,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
>  		goto out_err;
>  	}
>  
> -	/* Remove old maps */
> -	old_map = map_groups__first(kmaps, map->type);
> -	while (old_map) {
> -		struct map *next = map_groups__next(old_map);
> -
> -		if (old_map != map)
> -			map_groups__remove(kmaps, old_map);
> -		old_map = next;
> -	}
> +	old_mg = *kmaps;
> +	kmaps->maps[map->type].entries = RB_ROOT;
>  
>  	/* Find the kernel map using the first symbol */
>  	sym = dso__first_symbol(dso, map->type);
> @@ -1223,24 +1217,31 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
>  	while (!list_empty(&md.maps)) {
>  		new_map = list_entry(md.maps.next, struct map, node);
>  		list_del_init(&new_map->node);
> -		if (new_map == replacement_map) {
> -			map->start	= new_map->start;
> -			map->end	= new_map->end;
> -			map->pgoff	= new_map->pgoff;
> -			map->map_ip	= new_map->map_ip;
> -			map->unmap_ip	= new_map->unmap_ip;
> -			/* Ensure maps are correctly ordered */
> -			map__get(map);
> -			map_groups__remove(kmaps, map);
> -			map_groups__insert(kmaps, map);
> -			map__put(map);
> -		} else {
> -			map_groups__insert(kmaps, new_map);
> +
> +		map_groups__insert(kmaps, new_map);
> +
> +		if (new_map != replacement_map) {
> +			old_map = map_groups__find(&old_mg, new_map->type, new_map->start);
> +			if (old_map && dso__loaded(old_map->dso, old_map->type)) {
> +				new_map->pgoff = old_map->pgoff;
> +
> +				dso__put(new_map->dso);
> +				new_map->dso = dso__get(old_map->dso);
> +			}
>  		}
>  
>  		map__put(new_map);
>  	}
>  
> +	/* Remove old maps */
> +	old_map = map_groups__first(&old_mg, map->type);
> +	while (old_map) {
> +		struct map *next = map_groups__next(old_map);
> +
> +		map_groups__remove(&old_mg, old_map);
> +		old_map = next;
> +	}
> +
>  	/*
>  	 * Set the data type and long name so that kcore can be read via
>  	 * dso__data_read_addr().
> -- 
> 2.13.1