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: <20150908073747.GA2038@krava.brq.redhat.com>
Date:	Tue, 8 Sep 2015 09:37:47 +0200
From:	Jiri Olsa <jolsa@...hat.com>
To:	"Wangnan (F)" <wangnan0@...wei.com>
Cc:	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>,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH] perf report: Fix invalid memory accessing

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..

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/