linux-kernel - Re: [PATCH] perf report: Fix invalid memory accessing

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: <20150908151813.GF2435@redhat.com>
Date:	Tue, 8 Sep 2015 12:18:13 -0300
From:	Arnaldo Carvalho de Melo <acme@...hat.com>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	"Wangnan (F)" <wangnan0@...wei.com>, acme@...nel.org,
	kan.liang@...el.com, linux-kernel@...r.kernel.org,
	lizefan@...wei.com, pi3orama@....com,
	Adrian Hunter <adrian.hunter@...el.com>,
	Andi Kleen <ak@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH] perf report: Fix invalid memory accessing

Em Tue, Sep 08, 2015 at 09:37:47AM +0200, Jiri Olsa escreveu:
> On Mon, Sep 07, 2015 at 09:27:26PM +0800, Wangnan (F) wrote:
> 
> SNIP
> 
> > 
> > I found the problem.
> > 
> > perf relies on build_cpu_topology() to fetch CPU_TOPOLOGY from sysfs. It
> > depend on
> > the existance of
> > 
> > /sys/devices/system/cpu/cpu%d/topology/core_siblings_list
> > 
> > However, CPU can be canceled by hotcpu subsystem. After that the directory
> > of
> > /sys/devices/system/cpu/cpu%d/topology is gone, which causes perf's
> > write_cpu_topology() --> uild_cpu_topology() to fail, result in the above
> > perf.data.
> > 
> > So I think my patch is required.
> 
> no question there.. I just meant it should be placed in
> perf_event__preprocess_sample function with the rest of
> the 'al' initialization, like in the patch below?
> 
> it does not compile, because there're many places calling
> it and it'd need changing all callers to pass env, which
> seems to require more changes..

Humm, I think that we can have a pointer to the current perf_env, be it
from the current machine, or from the machine environment in the
perf.data file in struct machine, that way we don't need to change that
function prototype, I'm prototyping this now, will post a patch.

- Arnaldo
 
> also I'm not sure about removing:
> -	al->socket = cpu_map__get_socket_id(al->cpu);
> 
> 
> Does any command actually need this initialized from current system?
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
> index 0bf8c9889fc0..3339d2579bfc 100644
> --- a/tools/perf/util/event.c
> +++ b/tools/perf/util/event.c
> @@ -990,7 +990,8 @@ void thread__find_addr_location(struct thread *thread,
>  int perf_event__preprocess_sample(const union perf_event *event,
>  				  struct machine *machine,
>  				  struct addr_location *al,
> -				  struct perf_sample *sample)
> +				  struct perf_sample *sample,
> +				  struct perf_env *env)
>  {
>  	u8 cpumode = event->header.misc & PERF_RECORD_MISC_CPUMODE_MASK;
>  	struct thread *thread = machine__findnew_thread(machine, sample->pid,
> @@ -1021,7 +1022,10 @@ int perf_event__preprocess_sample(const union perf_event *event,
>  
>  	al->sym = NULL;
>  	al->cpu = sample->cpu;
> -	al->socket = cpu_map__get_socket_id(al->cpu);
> +
> +	al.socket = -1;
> +	if (env->cpu && al->cpu >= 0)
> +		al.socket = env->cpu[al->cpu].socket_id;
>  
>  	if (al->map) {
>  		struct dso *dso = al->map->dso;
--
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/