[PATCH 1/3] perf/sched: fix 'perf sched trace'

[PATCH 1/3] perf/sched: fix 'perf sched trace'

[Xiao Guangrong]

If we use 'perf sched trace', it will call symbol__init() again,
and can lead perf tool crash:

[root@localhost perf]# ./perf sched trace
*** glibc detected *** ./perf: free(): invalid next size (normal): 0x094c1898 ***
======= Backtrace: =========
/lib/libc.so.6[0xb7602404]
/lib/libc.so.6(cfree+0x96)[0xb76043b6]
./perf[0x80730fe]
./perf[0x8074c97]
./perf[0x805eb59]
./perf[0x80536fd]
./perf[0x804b618]
./perf[0x804bdc3]
/lib/libc.so.6(__libc_start_main+0xe5)[0xb75a9735]
./perf[0x804af81]
======= Memory map: ========
08048000-08158000 r-xp 00000000 fe:00 556831     /home/eric/....
08158000-08168000 rw-p 0010f000 fe:00 556831     /home/eric/...
08168000-085fe000 rw-p 00000000 00:00 0
094ab000-094cc000 rw-p 00000000 00:00 0          [heap]

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 tools/perf/builtin-sched.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 45c46c7..7481ebd 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1888,13 +1888,18 @@ static int __cmd_record(int argc, const char **argv)
 
 int cmd_sched(int argc, const char **argv, const char *prefix __used)
 {
-	symbol__init(0);
-
 	argc = parse_options(argc, argv, sched_options, sched_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (!argc)
 		usage_with_options(sched_usage, sched_options);
 
+	/*
+	 * Aliased to 'perf trace' for now:
+	 */
+	if (!strcmp(argv[0], "trace"))
+		return cmd_trace(argc, argv, prefix);
+
+	symbol__init(0);
 	if (!strncmp(argv[0], "rec", 3)) {
 		return __cmd_record(argc, argv);
 	} else if (!strncmp(argv[0], "lat", 3)) {
@@ -1918,11 +1923,6 @@ int cmd_sched(int argc, const char **argv, const char *prefix __used)
 				usage_with_options(replay_usage, replay_options);
 		}
 		__cmd_replay();
-	} else if (!strcmp(argv[0], "trace")) {
-		/*
-		 * Aliased to 'perf trace' for now:
-		 */
-		return cmd_trace(argc, argv, prefix);
 	} else {
 		usage_with_options(sched_usage, sched_options);
 	}
-- 
1.6.1.2

[PATCH 2/3] perf_event: fix for procing raw event

[Xiao Guangrong]

We use 'data.raw_data' parameter to call process_raw_event(), but
data.raw_data buffer not include data size. it can make perf tool
crash.

This bug is imported by:

Commit-ID:  180f95e29aa8782c019caa64ede2a28d8ab62564
Gitweb:     http://git.kernel.org/tip/180f95e29aa8782c019caa64ede2a28d8ab62564
Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
AuthorDate: Sun, 6 Dec 2009 20:08:24 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 6 Dec 2009 18:15:01 +0100

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 tools/perf/builtin-kmem.c  |   11 ++++++++---
 tools/perf/builtin-sched.c |   11 ++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index f218990..f84d7a3 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -289,13 +289,17 @@ static void process_free_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, u32 size, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
+	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
+	raw = malloc_or_die(sizeof(*raw)+size);
+	raw->size = size;
+	memcpy(raw->data, data, size);
+
 	type = trace_parse_common_type(raw->data);
 	event = trace_find_event(type);
 
@@ -345,7 +349,8 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+			  data.time, thread);
 
 	return 0;
 }
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 7481ebd..4655e16 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1570,13 +1570,17 @@ process_sched_migrate_task_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, u32 size, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
+	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
+	raw = malloc_or_die(sizeof(*raw)+size);
+	raw->size = size;
+	memcpy(raw->data, data, size);
+
 	type = trace_parse_common_type(raw->data);
 	event = trace_find_event(type);
 
@@ -1629,7 +1633,8 @@ static int process_sample_event(event_t *event)
 	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+			  data.time, thread);
 
 	return 0;
 }
-- 
1.6.1.2

[PATCH 3/3] perf_event: fix __dsos__write_buildid_table()

[Xiao Guangrong]

The remain buff size is 'len - pos->long_name_len - 1', not
'len - pos->long_name_len + 1'

This bug is imported by:
Commit-ID:  7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
Gitweb:     http://git.kernel.org/tip/7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
AuthorDate: Sun, 6 Dec 2009 20:10:49 +0900
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 6 Dec 2009 18:15:02 +0100

perf tools: Misc small fixes

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 tools/perf/util/header.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 08b6759..59a9c0b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -209,7 +209,7 @@ static int __dsos__write_buildid_table(struct list_head *head, int fd)
 		err = do_write(fd, pos->long_name, pos->long_name_len + 1);
 		if (err < 0)
 			return err;
-		err = do_write(fd, zero_buf, len - pos->long_name_len + 1);
+		err = do_write(fd, zero_buf, len - pos->long_name_len - 1);
 		if (err < 0)
 			return err;
 	}
-- 
1.6.1.2

Re: [PATCH 3/3] perf_event: fix __dsos__write_buildid_table()

[OGAWA Hirofumi]

Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> writes:

> The remain buff size is 'len - pos->long_name_len - 1', not
> 'len - pos->long_name_len + 1'
>
> This bug is imported by:
> Commit-ID:  7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
> Gitweb:     http://git.kernel.org/tip/7691b1ec2e4a8d4bd88dcf88b29792399ebe1c91
> Author:     OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
> AuthorDate: Sun, 6 Dec 2009 20:10:49 +0900
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Sun, 6 Dec 2009 18:15:02 +0100
>
> perf tools: Misc small fixes

Oops, I forgot parens. Thanks for fixing it.

> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> ---
>  tools/perf/util/header.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 08b6759..59a9c0b 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -209,7 +209,7 @@ static int __dsos__write_buildid_table(struct list_head *head, int fd)
>  		err = do_write(fd, pos->long_name, pos->long_name_len + 1);
>  		if (err < 0)
>  			return err;
> -		err = do_write(fd, zero_buf, len - pos->long_name_len + 1);
> +		err = do_write(fd, zero_buf, len - pos->long_name_len - 1);
>  		if (err < 0)
>  			return err;
>  	}

-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[tip:perf/urgent] perf_event: Fix __dsos__write_buildid_table()

[tip-bot for Xiao Guangrong]

Commit-ID:  d9541ed3241bb6c2b805d3ea0e87563cf2a0c5c3
Gitweb:     http://git.kernel.org/tip/d9541ed3241bb6c2b805d3ea0e87563cf2a0c5c3
Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
AuthorDate: Mon, 7 Dec 2009 12:07:15 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 7 Dec 2009 06:26:24 +0100

perf_event: Fix __dsos__write_buildid_table()

The remain buff size is 'len - pos->long_name_len - 1', not
'len - pos->long_name_len + 1'

This bug was introduced by commit 7691b1e ("perf tools: Misc small
fixes").

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4B1C7F73.80707@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/util/header.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 08b6759..59a9c0b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -209,7 +209,7 @@ static int __dsos__write_buildid_table(struct list_head *head, int fd)
 		err = do_write(fd, pos->long_name, pos->long_name_len + 1);
 		if (err < 0)
 			return err;
-		err = do_write(fd, zero_buf, len - pos->long_name_len + 1);
+		err = do_write(fd, zero_buf, len - pos->long_name_len - 1);
 		if (err < 0)
 			return err;
 	}

Re: [PATCH 2/3] perf_event: fix for procing raw event

[Xiao Guangrong]

Xiao Guangrong wrote:
> We use 'data.raw_data' parameter to call process_raw_event(), but
> data.raw_data buffer not include data size. it can make perf tool
> crash.
> 

It seems raw->size is not use, I'll send a patch to cleanup it.

[PATCH] perf_event: fix for processing raw event - fix

[Xiao Guangrong]

raw->size is not used, this patch just cleanup it

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 tools/perf/builtin-kmem.c  |   38 +++++++-----------
 tools/perf/builtin-sched.c |   94 +++++++++++++++++++------------------------
 2 files changed, 56 insertions(+), 76 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index f84d7a3..7551a5f 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -57,11 +57,6 @@ static struct rb_root root_caller_sorted;
 static unsigned long total_requested, total_allocated;
 static unsigned long nr_allocs, nr_cross_allocs;
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define PATH_SYS_NODE	"/sys/devices/system/node"
 
 static void init_cpunode_map(void)
@@ -201,7 +196,7 @@ static void insert_caller_stat(unsigned long call_site,
 	}
 }
 
-static void process_alloc_event(struct raw_event_sample *raw,
+static void process_alloc_event(void *data,
 				struct event *event,
 				int cpu,
 				u64 timestamp __used,
@@ -214,10 +209,10 @@ static void process_alloc_event(struct raw_event_sample *raw,
 	int bytes_alloc;
 	int node1, node2;
 
-	ptr = raw_field_value(event, "ptr", raw->data);
-	call_site = raw_field_value(event, "call_site", raw->data);
-	bytes_req = raw_field_value(event, "bytes_req", raw->data);
-	bytes_alloc = raw_field_value(event, "bytes_alloc", raw->data);
+	ptr = raw_field_value(event, "ptr", data);
+	call_site = raw_field_value(event, "call_site", data);
+	bytes_req = raw_field_value(event, "bytes_req", data);
+	bytes_alloc = raw_field_value(event, "bytes_alloc", data);
 
 	insert_alloc_stat(call_site, ptr, bytes_req, bytes_alloc, cpu);
 	insert_caller_stat(call_site, bytes_req, bytes_alloc);
@@ -227,7 +222,7 @@ static void process_alloc_event(struct raw_event_sample *raw,
 
 	if (node) {
 		node1 = cpunode_map[cpu];
-		node2 = raw_field_value(event, "node", raw->data);
+		node2 = raw_field_value(event, "node", data);
 		if (node1 != node2)
 			nr_cross_allocs++;
 	}
@@ -262,7 +257,7 @@ static struct alloc_stat *search_alloc_stat(unsigned long ptr,
 	return NULL;
 }
 
-static void process_free_event(struct raw_event_sample *raw,
+static void process_free_event(void *data,
 			       struct event *event,
 			       int cpu,
 			       u64 timestamp __used,
@@ -271,7 +266,7 @@ static void process_free_event(struct raw_event_sample *raw,
 	unsigned long ptr;
 	struct alloc_stat *s_alloc, *s_caller;
 
-	ptr = raw_field_value(event, "ptr", raw->data);
+	ptr = raw_field_value(event, "ptr", data);
 
 	s_alloc = search_alloc_stat(ptr, 0, &root_alloc_stat, ptr_cmp);
 	if (!s_alloc)
@@ -289,35 +284,30 @@ static void process_free_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, u32 size, void *data,
+process_raw_event(event_t *raw_event __used, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
-	raw = malloc_or_die(sizeof(*raw)+size);
-	raw->size = size;
-	memcpy(raw->data, data, size);
-
-	type = trace_parse_common_type(raw->data);
+	type = trace_parse_common_type(data);
 	event = trace_find_event(type);
 
 	if (!strcmp(event->name, "kmalloc") ||
 	    !strcmp(event->name, "kmem_cache_alloc")) {
-		process_alloc_event(raw, event, cpu, timestamp, thread, 0);
+		process_alloc_event(data, event, cpu, timestamp, thread, 0);
 		return;
 	}
 
 	if (!strcmp(event->name, "kmalloc_node") ||
 	    !strcmp(event->name, "kmem_cache_alloc_node")) {
-		process_alloc_event(raw, event, cpu, timestamp, thread, 1);
+		process_alloc_event(data, event, cpu, timestamp, thread, 1);
 		return;
 	}
 
 	if (!strcmp(event->name, "kfree") ||
 	    !strcmp(event->name, "kmem_cache_free")) {
-		process_free_event(raw, event, cpu, timestamp, thread);
+		process_free_event(data, event, cpu, timestamp, thread);
 		return;
 	}
 }
@@ -349,7 +339,7 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+	process_raw_event(event, data.raw_data, data.cpu,
 			  data.time, thread);
 
 	return 0;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4655e16..19f43fa 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -628,11 +628,6 @@ static void test_calibrations(void)
 	printf("the sleep test took %Ld nsecs\n", T1-T0);
 }
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define FILL_FIELD(ptr, field, event, data)	\
 	ptr.field = (typeof(ptr.field)) raw_field_value(event, #field, data)
 
@@ -1356,7 +1351,7 @@ static void sort_lat(void)
 static struct trace_sched_handler *trace_handler;
 
 static void
-process_sched_wakeup_event(struct raw_event_sample *raw,
+process_sched_wakeup_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1364,13 +1359,13 @@ process_sched_wakeup_event(struct raw_event_sample *raw,
 {
 	struct trace_wakeup_event wakeup_event;
 
-	FILL_COMMON_FIELDS(wakeup_event, event, raw->data);
+	FILL_COMMON_FIELDS(wakeup_event, event, data);
 
-	FILL_ARRAY(wakeup_event, comm, event, raw->data);
-	FILL_FIELD(wakeup_event, pid, event, raw->data);
-	FILL_FIELD(wakeup_event, prio, event, raw->data);
-	FILL_FIELD(wakeup_event, success, event, raw->data);
-	FILL_FIELD(wakeup_event, cpu, event, raw->data);
+	FILL_ARRAY(wakeup_event, comm, event, data);
+	FILL_FIELD(wakeup_event, pid, event, data);
+	FILL_FIELD(wakeup_event, prio, event, data);
+	FILL_FIELD(wakeup_event, success, event, data);
+	FILL_FIELD(wakeup_event, cpu, event, data);
 
 	if (trace_handler->wakeup_event)
 		trace_handler->wakeup_event(&wakeup_event, event, cpu, timestamp, thread);
@@ -1469,7 +1464,7 @@ map_switch_event(struct trace_switch_event *switch_event,
 
 
 static void
-process_sched_switch_event(struct raw_event_sample *raw,
+process_sched_switch_event(void *data,
 			   struct event *event,
 			   int this_cpu,
 			   u64 timestamp __used,
@@ -1477,15 +1472,15 @@ process_sched_switch_event(struct raw_event_sample *raw,
 {
 	struct trace_switch_event switch_event;
 
-	FILL_COMMON_FIELDS(switch_event, event, raw->data);
+	FILL_COMMON_FIELDS(switch_event, event, data);
 
-	FILL_ARRAY(switch_event, prev_comm, event, raw->data);
-	FILL_FIELD(switch_event, prev_pid, event, raw->data);
-	FILL_FIELD(switch_event, prev_prio, event, raw->data);
-	FILL_FIELD(switch_event, prev_state, event, raw->data);
-	FILL_ARRAY(switch_event, next_comm, event, raw->data);
-	FILL_FIELD(switch_event, next_pid, event, raw->data);
-	FILL_FIELD(switch_event, next_prio, event, raw->data);
+	FILL_ARRAY(switch_event, prev_comm, event, data);
+	FILL_FIELD(switch_event, prev_pid, event, data);
+	FILL_FIELD(switch_event, prev_prio, event, data);
+	FILL_FIELD(switch_event, prev_state, event, data);
+	FILL_ARRAY(switch_event, next_comm, event, data);
+	FILL_FIELD(switch_event, next_pid, event, data);
+	FILL_FIELD(switch_event, next_prio, event, data);
 
 	if (curr_pid[this_cpu] != (u32)-1) {
 		/*
@@ -1502,7 +1497,7 @@ process_sched_switch_event(struct raw_event_sample *raw,
 }
 
 static void
-process_sched_runtime_event(struct raw_event_sample *raw,
+process_sched_runtime_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1510,17 +1505,17 @@ process_sched_runtime_event(struct raw_event_sample *raw,
 {
 	struct trace_runtime_event runtime_event;
 
-	FILL_ARRAY(runtime_event, comm, event, raw->data);
-	FILL_FIELD(runtime_event, pid, event, raw->data);
-	FILL_FIELD(runtime_event, runtime, event, raw->data);
-	FILL_FIELD(runtime_event, vruntime, event, raw->data);
+	FILL_ARRAY(runtime_event, comm, event, data);
+	FILL_FIELD(runtime_event, pid, event, data);
+	FILL_FIELD(runtime_event, runtime, event, data);
+	FILL_FIELD(runtime_event, vruntime, event, data);
 
 	if (trace_handler->runtime_event)
 		trace_handler->runtime_event(&runtime_event, event, cpu, timestamp, thread);
 }
 
 static void
-process_sched_fork_event(struct raw_event_sample *raw,
+process_sched_fork_event(void *data,
 			 struct event *event,
 			 int cpu __used,
 			 u64 timestamp __used,
@@ -1528,12 +1523,12 @@ process_sched_fork_event(struct raw_event_sample *raw,
 {
 	struct trace_fork_event fork_event;
 
-	FILL_COMMON_FIELDS(fork_event, event, raw->data);
+	FILL_COMMON_FIELDS(fork_event, event, data);
 
-	FILL_ARRAY(fork_event, parent_comm, event, raw->data);
-	FILL_FIELD(fork_event, parent_pid, event, raw->data);
-	FILL_ARRAY(fork_event, child_comm, event, raw->data);
-	FILL_FIELD(fork_event, child_pid, event, raw->data);
+	FILL_ARRAY(fork_event, parent_comm, event, data);
+	FILL_FIELD(fork_event, parent_pid, event, data);
+	FILL_ARRAY(fork_event, child_comm, event, data);
+	FILL_FIELD(fork_event, child_pid, event, data);
 
 	if (trace_handler->fork_event)
 		trace_handler->fork_event(&fork_event, event, cpu, timestamp, thread);
@@ -1550,7 +1545,7 @@ process_sched_exit_event(struct event *event,
 }
 
 static void
-process_sched_migrate_task_event(struct raw_event_sample *raw,
+process_sched_migrate_task_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1558,46 +1553,42 @@ process_sched_migrate_task_event(struct raw_event_sample *raw,
 {
 	struct trace_migrate_task_event migrate_task_event;
 
-	FILL_COMMON_FIELDS(migrate_task_event, event, raw->data);
+	FILL_COMMON_FIELDS(migrate_task_event, event, data);
 
-	FILL_ARRAY(migrate_task_event, comm, event, raw->data);
-	FILL_FIELD(migrate_task_event, pid, event, raw->data);
-	FILL_FIELD(migrate_task_event, prio, event, raw->data);
-	FILL_FIELD(migrate_task_event, cpu, event, raw->data);
+	FILL_ARRAY(migrate_task_event, comm, event, data);
+	FILL_FIELD(migrate_task_event, pid, event, data);
+	FILL_FIELD(migrate_task_event, prio, event, data);
+	FILL_FIELD(migrate_task_event, cpu, event, data);
 
 	if (trace_handler->migrate_task_event)
 		trace_handler->migrate_task_event(&migrate_task_event, event, cpu, timestamp, thread);
 }
 
 static void
-process_raw_event(event_t *raw_event __used, u32 size, void *data,
+process_raw_event(event_t *raw_event __used, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
-	raw = malloc_or_die(sizeof(*raw)+size);
-	raw->size = size;
-	memcpy(raw->data, data, size);
 
-	type = trace_parse_common_type(raw->data);
+	type = trace_parse_common_type(data);
 	event = trace_find_event(type);
 
 	if (!strcmp(event->name, "sched_switch"))
-		process_sched_switch_event(raw, event, cpu, timestamp, thread);
+		process_sched_switch_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_stat_runtime"))
-		process_sched_runtime_event(raw, event, cpu, timestamp, thread);
+		process_sched_runtime_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_wakeup"))
-		process_sched_wakeup_event(raw, event, cpu, timestamp, thread);
+		process_sched_wakeup_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_wakeup_new"))
-		process_sched_wakeup_event(raw, event, cpu, timestamp, thread);
+		process_sched_wakeup_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_process_fork"))
-		process_sched_fork_event(raw, event, cpu, timestamp, thread);
+		process_sched_fork_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_process_exit"))
 		process_sched_exit_event(event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_migrate_task"))
-		process_sched_migrate_task_event(raw, event, cpu, timestamp, thread);
+		process_sched_migrate_task_event(data, event, cpu, timestamp, thread);
 }
 
 static int process_sample_event(event_t *event)
@@ -1633,8 +1624,7 @@ static int process_sample_event(event_t *event)
 	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
-			  data.time, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }
-- 
1.6.1.2

[tip:perf/urgent] perf_event: Eliminate raw->size

[tip-bot for Xiao Guangrong]

Commit-ID:  f48f669d42e133db839af16656fd720107ef6742
Gitweb:     http://git.kernel.org/tip/f48f669d42e133db839af16656fd720107ef6742
Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
AuthorDate: Mon, 7 Dec 2009 13:04:04 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 7 Dec 2009 06:26:25 +0100

perf_event: Eliminate raw->size

raw->size is not used, this patch just cleans it up.

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4B1C8CC4.4050007@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-kmem.c  |   38 +++++++-----------
 tools/perf/builtin-sched.c |   94 +++++++++++++++++++------------------------
 2 files changed, 56 insertions(+), 76 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index f84d7a3..7551a5f 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -57,11 +57,6 @@ static struct rb_root root_caller_sorted;
 static unsigned long total_requested, total_allocated;
 static unsigned long nr_allocs, nr_cross_allocs;
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define PATH_SYS_NODE	"/sys/devices/system/node"
 
 static void init_cpunode_map(void)
@@ -201,7 +196,7 @@ static void insert_caller_stat(unsigned long call_site,
 	}
 }
 
-static void process_alloc_event(struct raw_event_sample *raw,
+static void process_alloc_event(void *data,
 				struct event *event,
 				int cpu,
 				u64 timestamp __used,
@@ -214,10 +209,10 @@ static void process_alloc_event(struct raw_event_sample *raw,
 	int bytes_alloc;
 	int node1, node2;
 
-	ptr = raw_field_value(event, "ptr", raw->data);
-	call_site = raw_field_value(event, "call_site", raw->data);
-	bytes_req = raw_field_value(event, "bytes_req", raw->data);
-	bytes_alloc = raw_field_value(event, "bytes_alloc", raw->data);
+	ptr = raw_field_value(event, "ptr", data);
+	call_site = raw_field_value(event, "call_site", data);
+	bytes_req = raw_field_value(event, "bytes_req", data);
+	bytes_alloc = raw_field_value(event, "bytes_alloc", data);
 
 	insert_alloc_stat(call_site, ptr, bytes_req, bytes_alloc, cpu);
 	insert_caller_stat(call_site, bytes_req, bytes_alloc);
@@ -227,7 +222,7 @@ static void process_alloc_event(struct raw_event_sample *raw,
 
 	if (node) {
 		node1 = cpunode_map[cpu];
-		node2 = raw_field_value(event, "node", raw->data);
+		node2 = raw_field_value(event, "node", data);
 		if (node1 != node2)
 			nr_cross_allocs++;
 	}
@@ -262,7 +257,7 @@ static struct alloc_stat *search_alloc_stat(unsigned long ptr,
 	return NULL;
 }
 
-static void process_free_event(struct raw_event_sample *raw,
+static void process_free_event(void *data,
 			       struct event *event,
 			       int cpu,
 			       u64 timestamp __used,
@@ -271,7 +266,7 @@ static void process_free_event(struct raw_event_sample *raw,
 	unsigned long ptr;
 	struct alloc_stat *s_alloc, *s_caller;
 
-	ptr = raw_field_value(event, "ptr", raw->data);
+	ptr = raw_field_value(event, "ptr", data);
 
 	s_alloc = search_alloc_stat(ptr, 0, &root_alloc_stat, ptr_cmp);
 	if (!s_alloc)
@@ -289,35 +284,30 @@ static void process_free_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, u32 size, void *data,
+process_raw_event(event_t *raw_event __used, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
-	raw = malloc_or_die(sizeof(*raw)+size);
-	raw->size = size;
-	memcpy(raw->data, data, size);
-
-	type = trace_parse_common_type(raw->data);
+	type = trace_parse_common_type(data);
 	event = trace_find_event(type);
 
 	if (!strcmp(event->name, "kmalloc") ||
 	    !strcmp(event->name, "kmem_cache_alloc")) {
-		process_alloc_event(raw, event, cpu, timestamp, thread, 0);
+		process_alloc_event(data, event, cpu, timestamp, thread, 0);
 		return;
 	}
 
 	if (!strcmp(event->name, "kmalloc_node") ||
 	    !strcmp(event->name, "kmem_cache_alloc_node")) {
-		process_alloc_event(raw, event, cpu, timestamp, thread, 1);
+		process_alloc_event(data, event, cpu, timestamp, thread, 1);
 		return;
 	}
 
 	if (!strcmp(event->name, "kfree") ||
 	    !strcmp(event->name, "kmem_cache_free")) {
-		process_free_event(raw, event, cpu, timestamp, thread);
+		process_free_event(data, event, cpu, timestamp, thread);
 		return;
 	}
 }
@@ -349,7 +339,7 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+	process_raw_event(event, data.raw_data, data.cpu,
 			  data.time, thread);
 
 	return 0;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 4655e16..19f43fa 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -628,11 +628,6 @@ static void test_calibrations(void)
 	printf("the sleep test took %Ld nsecs\n", T1-T0);
 }
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define FILL_FIELD(ptr, field, event, data)	\
 	ptr.field = (typeof(ptr.field)) raw_field_value(event, #field, data)
 
@@ -1356,7 +1351,7 @@ static void sort_lat(void)
 static struct trace_sched_handler *trace_handler;
 
 static void
-process_sched_wakeup_event(struct raw_event_sample *raw,
+process_sched_wakeup_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1364,13 +1359,13 @@ process_sched_wakeup_event(struct raw_event_sample *raw,
 {
 	struct trace_wakeup_event wakeup_event;
 
-	FILL_COMMON_FIELDS(wakeup_event, event, raw->data);
+	FILL_COMMON_FIELDS(wakeup_event, event, data);
 
-	FILL_ARRAY(wakeup_event, comm, event, raw->data);
-	FILL_FIELD(wakeup_event, pid, event, raw->data);
-	FILL_FIELD(wakeup_event, prio, event, raw->data);
-	FILL_FIELD(wakeup_event, success, event, raw->data);
-	FILL_FIELD(wakeup_event, cpu, event, raw->data);
+	FILL_ARRAY(wakeup_event, comm, event, data);
+	FILL_FIELD(wakeup_event, pid, event, data);
+	FILL_FIELD(wakeup_event, prio, event, data);
+	FILL_FIELD(wakeup_event, success, event, data);
+	FILL_FIELD(wakeup_event, cpu, event, data);
 
 	if (trace_handler->wakeup_event)
 		trace_handler->wakeup_event(&wakeup_event, event, cpu, timestamp, thread);
@@ -1469,7 +1464,7 @@ map_switch_event(struct trace_switch_event *switch_event,
 
 
 static void
-process_sched_switch_event(struct raw_event_sample *raw,
+process_sched_switch_event(void *data,
 			   struct event *event,
 			   int this_cpu,
 			   u64 timestamp __used,
@@ -1477,15 +1472,15 @@ process_sched_switch_event(struct raw_event_sample *raw,
 {
 	struct trace_switch_event switch_event;
 
-	FILL_COMMON_FIELDS(switch_event, event, raw->data);
+	FILL_COMMON_FIELDS(switch_event, event, data);
 
-	FILL_ARRAY(switch_event, prev_comm, event, raw->data);
-	FILL_FIELD(switch_event, prev_pid, event, raw->data);
-	FILL_FIELD(switch_event, prev_prio, event, raw->data);
-	FILL_FIELD(switch_event, prev_state, event, raw->data);
-	FILL_ARRAY(switch_event, next_comm, event, raw->data);
-	FILL_FIELD(switch_event, next_pid, event, raw->data);
-	FILL_FIELD(switch_event, next_prio, event, raw->data);
+	FILL_ARRAY(switch_event, prev_comm, event, data);
+	FILL_FIELD(switch_event, prev_pid, event, data);
+	FILL_FIELD(switch_event, prev_prio, event, data);
+	FILL_FIELD(switch_event, prev_state, event, data);
+	FILL_ARRAY(switch_event, next_comm, event, data);
+	FILL_FIELD(switch_event, next_pid, event, data);
+	FILL_FIELD(switch_event, next_prio, event, data);
 
 	if (curr_pid[this_cpu] != (u32)-1) {
 		/*
@@ -1502,7 +1497,7 @@ process_sched_switch_event(struct raw_event_sample *raw,
 }
 
 static void
-process_sched_runtime_event(struct raw_event_sample *raw,
+process_sched_runtime_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1510,17 +1505,17 @@ process_sched_runtime_event(struct raw_event_sample *raw,
 {
 	struct trace_runtime_event runtime_event;
 
-	FILL_ARRAY(runtime_event, comm, event, raw->data);
-	FILL_FIELD(runtime_event, pid, event, raw->data);
-	FILL_FIELD(runtime_event, runtime, event, raw->data);
-	FILL_FIELD(runtime_event, vruntime, event, raw->data);
+	FILL_ARRAY(runtime_event, comm, event, data);
+	FILL_FIELD(runtime_event, pid, event, data);
+	FILL_FIELD(runtime_event, runtime, event, data);
+	FILL_FIELD(runtime_event, vruntime, event, data);
 
 	if (trace_handler->runtime_event)
 		trace_handler->runtime_event(&runtime_event, event, cpu, timestamp, thread);
 }
 
 static void
-process_sched_fork_event(struct raw_event_sample *raw,
+process_sched_fork_event(void *data,
 			 struct event *event,
 			 int cpu __used,
 			 u64 timestamp __used,
@@ -1528,12 +1523,12 @@ process_sched_fork_event(struct raw_event_sample *raw,
 {
 	struct trace_fork_event fork_event;
 
-	FILL_COMMON_FIELDS(fork_event, event, raw->data);
+	FILL_COMMON_FIELDS(fork_event, event, data);
 
-	FILL_ARRAY(fork_event, parent_comm, event, raw->data);
-	FILL_FIELD(fork_event, parent_pid, event, raw->data);
-	FILL_ARRAY(fork_event, child_comm, event, raw->data);
-	FILL_FIELD(fork_event, child_pid, event, raw->data);
+	FILL_ARRAY(fork_event, parent_comm, event, data);
+	FILL_FIELD(fork_event, parent_pid, event, data);
+	FILL_ARRAY(fork_event, child_comm, event, data);
+	FILL_FIELD(fork_event, child_pid, event, data);
 
 	if (trace_handler->fork_event)
 		trace_handler->fork_event(&fork_event, event, cpu, timestamp, thread);
@@ -1550,7 +1545,7 @@ process_sched_exit_event(struct event *event,
 }
 
 static void
-process_sched_migrate_task_event(struct raw_event_sample *raw,
+process_sched_migrate_task_event(void *data,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1558,46 +1553,42 @@ process_sched_migrate_task_event(struct raw_event_sample *raw,
 {
 	struct trace_migrate_task_event migrate_task_event;
 
-	FILL_COMMON_FIELDS(migrate_task_event, event, raw->data);
+	FILL_COMMON_FIELDS(migrate_task_event, event, data);
 
-	FILL_ARRAY(migrate_task_event, comm, event, raw->data);
-	FILL_FIELD(migrate_task_event, pid, event, raw->data);
-	FILL_FIELD(migrate_task_event, prio, event, raw->data);
-	FILL_FIELD(migrate_task_event, cpu, event, raw->data);
+	FILL_ARRAY(migrate_task_event, comm, event, data);
+	FILL_FIELD(migrate_task_event, pid, event, data);
+	FILL_FIELD(migrate_task_event, prio, event, data);
+	FILL_FIELD(migrate_task_event, cpu, event, data);
 
 	if (trace_handler->migrate_task_event)
 		trace_handler->migrate_task_event(&migrate_task_event, event, cpu, timestamp, thread);
 }
 
 static void
-process_raw_event(event_t *raw_event __used, u32 size, void *data,
+process_raw_event(event_t *raw_event __used, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
-	raw = malloc_or_die(sizeof(*raw)+size);
-	raw->size = size;
-	memcpy(raw->data, data, size);
 
-	type = trace_parse_common_type(raw->data);
+	type = trace_parse_common_type(data);
 	event = trace_find_event(type);
 
 	if (!strcmp(event->name, "sched_switch"))
-		process_sched_switch_event(raw, event, cpu, timestamp, thread);
+		process_sched_switch_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_stat_runtime"))
-		process_sched_runtime_event(raw, event, cpu, timestamp, thread);
+		process_sched_runtime_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_wakeup"))
-		process_sched_wakeup_event(raw, event, cpu, timestamp, thread);
+		process_sched_wakeup_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_wakeup_new"))
-		process_sched_wakeup_event(raw, event, cpu, timestamp, thread);
+		process_sched_wakeup_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_process_fork"))
-		process_sched_fork_event(raw, event, cpu, timestamp, thread);
+		process_sched_fork_event(data, event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_process_exit"))
 		process_sched_exit_event(event, cpu, timestamp, thread);
 	if (!strcmp(event->name, "sched_migrate_task"))
-		process_sched_migrate_task_event(raw, event, cpu, timestamp, thread);
+		process_sched_migrate_task_event(data, event, cpu, timestamp, thread);
 }
 
 static int process_sample_event(event_t *event)
@@ -1633,8 +1624,7 @@ static int process_sample_event(event_t *event)
 	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
-			  data.time, thread);
+	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
 
 	return 0;
 }

Re: [tip:perf/urgent] perf_event: Eliminate raw->size

[Peter Zijlstra]

On Mon, 2009-12-07 at 05:31 +0000, tip-bot for Xiao Guangrong wrote:
> Commit-ID:  f48f669d42e133db839af16656fd720107ef6742
> Gitweb:     http://git.kernel.org/tip/f48f669d42e133db839af16656fd720107ef6742
> Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> AuthorDate: Mon, 7 Dec 2009 13:04:04 +0800
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Mon, 7 Dec 2009 06:26:25 +0100
> 
> perf_event: Eliminate raw->size
> 
> raw->size is not used, this patch just cleans it up.

This patch feels wrong.. I would suspect it to be used to validate the
data parsing, like do we consume past the end and did we consume
everything.

Re: [tip:perf/urgent] perf_event: Eliminate raw->size

[Xiao Guangrong]

Peter Zijlstra wrote:
> On Mon, 2009-12-07 at 05:31 +0000, tip-bot for Xiao Guangrong wrote:
>> Commit-ID:  f48f669d42e133db839af16656fd720107ef6742
>> Gitweb:     http://git.kernel.org/tip/f48f669d42e133db839af16656fd720107ef6742
>> Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
>> AuthorDate: Mon, 7 Dec 2009 13:04:04 +0800
>> Committer:  Ingo Molnar <mingo@elte.hu>
>> CommitDate: Mon, 7 Dec 2009 06:26:25 +0100
>>
>> perf_event: Eliminate raw->size
>>
>> raw->size is not used, this patch just cleans it up.
> 
> This patch feels wrong.. I would suspect it to be used to validate the
> data parsing, like do we consume past the end and did we consume
> everything.
> 

But as i noticed in current code, it just use raw->data, maybe i miss something?
what i do is just using 'data' instead of 'raw->data' :-(

Thanks,
Xiao

> 
> 

Re: [tip:perf/urgent] perf_event: Eliminate raw->size

[OGAWA Hirofumi]

Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> writes:

> Peter Zijlstra wrote:
>> On Mon, 2009-12-07 at 05:31 +0000, tip-bot for Xiao Guangrong wrote:
>>> Commit-ID:  f48f669d42e133db839af16656fd720107ef6742
>>> Gitweb:     http://git.kernel.org/tip/f48f669d42e133db839af16656fd720107ef6742
>>> Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
>>> AuthorDate: Mon, 7 Dec 2009 13:04:04 +0800
>>> Committer:  Ingo Molnar <mingo@elte.hu>
>>> CommitDate: Mon, 7 Dec 2009 06:26:25 +0100
>>>
>>> perf_event: Eliminate raw->size
>>>
>>> raw->size is not used, this patch just cleans it up.
>> 
>> This patch feels wrong.. I would suspect it to be used to validate the
>> data parsing, like do we consume past the end and did we consume
>> everything.
>> 
>
> But as i noticed in current code, it just use raw->data, maybe i miss something?
> what i do is just using 'data' instead of 'raw->data' :-(

Well, it's not user fault at all though. To pass "struct sample_raw_data"
instead of "void *" to raw_field_value/raw_field_ptr/etc., then bounds
check of sample_raw_data would be sane. It is what I intended...

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] perf_event: fix for processing raw event - fix

[OGAWA Hirofumi]

Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> writes:

> raw->size is not used, this patch just cleanup it

Oops, I didn't notice those. Thanks.

> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> ---
>  tools/perf/builtin-kmem.c  |   38 +++++++-----------
>  tools/perf/builtin-sched.c |   94 +++++++++++++++++++------------------------
>  2 files changed, 56 insertions(+), 76 deletions(-)
>
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index f84d7a3..7551a5f 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -57,11 +57,6 @@ static struct rb_root root_caller_sorted;
>  static unsigned long total_requested, total_allocated;
>  static unsigned long nr_allocs, nr_cross_allocs;
>  
> -struct raw_event_sample {
> -	u32 size;
> -	char data[0];
> -};
> -
>  #define PATH_SYS_NODE	"/sys/devices/system/node"
>  
>  static void init_cpunode_map(void)
> @@ -201,7 +196,7 @@ static void insert_caller_stat(unsigned long call_site,
>  	}
>  }
>  
> -static void process_alloc_event(struct raw_event_sample *raw,
> +static void process_alloc_event(void *data,
>  				struct event *event,
>  				int cpu,
>  				u64 timestamp __used,

How about the following patch instead of playing with unsafe "void *"?
With this, I guess it does type check, and filed handling functions can
check "size" by passing "struct sample_raw_data" instead of "void *" in
future.

[Sorry, this patch was almost compiled only, and tested slightly.]

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>



Signed-off-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
---

 tools/perf/builtin-kmem.c      |   14 ++++----------
 tools/perf/builtin-sched.c     |   20 +++++++-------------
 tools/perf/builtin-timechart.c |    4 ++--
 tools/perf/builtin-trace.c     |    4 ++--
 tools/perf/util/event.c        |    8 ++------
 tools/perf/util/event.h        |    8 ++++++--
 6 files changed, 23 insertions(+), 35 deletions(-)

diff -puN tools/perf/util/event.h~perf-raw_sample tools/perf/util/event.h
--- linux-2.6/tools/perf/util/event.h~perf-raw_sample	2009-12-07 14:48:09.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/event.h	2009-12-07 15:25:51.000000000 +0900
@@ -61,6 +61,11 @@ struct sample_event {
 	u64 array[];
 };
 
+struct sample_raw_data {
+	u32 size;
+	unsigned char data[];
+};
+
 struct sample_data {
 	u64 ip;
 	u32 pid, tid;
@@ -71,8 +76,7 @@ struct sample_data {
 	u32 cpu;
 	u64 period;
 	struct ip_callchain *callchain;
-	u32 raw_size;
-	void *raw_data;
+	struct sample_raw_data *raw;
 };
 
 #define BUILD_ID_SIZE 20
diff -puN tools/perf/builtin-sched.c~perf-raw_sample tools/perf/builtin-sched.c
--- linux-2.6/tools/perf/builtin-sched.c~perf-raw_sample	2009-12-07 14:49:09.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-sched.c	2009-12-07 14:58:04.000000000 +0900
@@ -628,11 +628,6 @@ static void test_calibrations(void)
 	printf("the sleep test took %Ld nsecs\n", T1-T0);
 }
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define FILL_FIELD(ptr, field, event, data)	\
 	ptr.field = (typeof(ptr.field)) raw_field_value(event, #field, data)
 
@@ -1356,7 +1351,7 @@ static void sort_lat(void)
 static struct trace_sched_handler *trace_handler;
 
 static void
-process_sched_wakeup_event(struct raw_event_sample *raw,
+process_sched_wakeup_event(struct sample_raw_data *raw,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1469,7 +1464,7 @@ map_switch_event(struct trace_switch_eve
 
 
 static void
-process_sched_switch_event(struct raw_event_sample *raw,
+process_sched_switch_event(struct sample_raw_data *raw,
 			   struct event *event,
 			   int this_cpu,
 			   u64 timestamp __used,
@@ -1502,7 +1497,7 @@ process_sched_switch_event(struct raw_ev
 }
 
 static void
-process_sched_runtime_event(struct raw_event_sample *raw,
+process_sched_runtime_event(struct sample_raw_data *raw,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1520,7 +1515,7 @@ process_sched_runtime_event(struct raw_e
 }
 
 static void
-process_sched_fork_event(struct raw_event_sample *raw,
+process_sched_fork_event(struct sample_raw_data *raw,
 			 struct event *event,
 			 int cpu __used,
 			 u64 timestamp __used,
@@ -1550,7 +1545,7 @@ process_sched_exit_event(struct event *e
 }
 
 static void
-process_sched_migrate_task_event(struct raw_event_sample *raw,
+process_sched_migrate_task_event(struct sample_raw_data *raw,
 			   struct event *event,
 			   int cpu __used,
 			   u64 timestamp __used,
@@ -1570,10 +1565,9 @@ process_sched_migrate_task_event(struct 
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, struct sample_raw_data *raw,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
 	struct event *event;
 	int type;
 
@@ -1629,7 +1623,7 @@ static int process_sample_event(event_t 
 	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw, data.cpu, data.time, thread);
 
 	return 0;
 }
diff -puN tools/perf/builtin-trace.c~perf-raw_sample tools/perf/builtin-trace.c
--- linux-2.6/tools/perf/builtin-trace.c~perf-raw_sample	2009-12-07 14:50:25.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-trace.c	2009-12-07 14:50:53.000000000 +0900
@@ -95,8 +95,8 @@ static int process_sample_event(event_t 
 		 * field, although it should be the same than this perf
 		 * event pid
 		 */
-		scripting_ops->process_event(data.cpu, data.raw_data,
-					     data.raw_size,
+		scripting_ops->process_event(data.cpu, data.raw->data,
+					     data.raw->size,
 					     data.time, thread->comm);
 	}
 	event__stats.total += data.period;
diff -puN tools/perf/builtin-timechart.c~perf-raw_sample tools/perf/builtin-timechart.c
--- linux-2.6/tools/perf/builtin-timechart.c~perf-raw_sample	2009-12-07 14:51:05.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-timechart.c	2009-12-07 14:51:26.000000000 +0900
@@ -497,8 +497,8 @@ process_sample_event(event_t *event)
 			last_time = data.time;
 	}
 
-	te = (void *)data.raw_data;
-	if (sample_type & PERF_SAMPLE_RAW && data.raw_size > 0) {
+	te = (void *)data.raw->data;
+	if (sample_type & PERF_SAMPLE_RAW && data.raw->size > 0) {
 		char *event_str;
 		struct power_entry *pe;
 
diff -puN tools/perf/builtin-kmem.c~perf-raw_sample tools/perf/builtin-kmem.c
--- linux-2.6/tools/perf/builtin-kmem.c~perf-raw_sample	2009-12-07 14:51:38.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/builtin-kmem.c	2009-12-07 14:53:57.000000000 +0900
@@ -57,11 +57,6 @@ static struct rb_root root_caller_sorted
 static unsigned long total_requested, total_allocated;
 static unsigned long nr_allocs, nr_cross_allocs;
 
-struct raw_event_sample {
-	u32 size;
-	char data[0];
-};
-
 #define PATH_SYS_NODE	"/sys/devices/system/node"
 
 static void init_cpunode_map(void)
@@ -201,7 +196,7 @@ static void insert_caller_stat(unsigned 
 	}
 }
 
-static void process_alloc_event(struct raw_event_sample *raw,
+static void process_alloc_event(struct sample_raw_data *raw,
 				struct event *event,
 				int cpu,
 				u64 timestamp __used,
@@ -262,7 +257,7 @@ static struct alloc_stat *search_alloc_s
 	return NULL;
 }
 
-static void process_free_event(struct raw_event_sample *raw,
+static void process_free_event(struct sample_raw_data *raw,
 			       struct event *event,
 			       int cpu,
 			       u64 timestamp __used,
@@ -289,10 +284,9 @@ static void process_free_event(struct ra
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, struct sample_raw_data *raw,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
 	struct event *event;
 	int type;
 
@@ -345,7 +339,7 @@ static int process_sample_event(event_t 
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw, data.cpu, data.time, thread);
 
 	return 0;
 }
diff -puN tools/perf/util/event.c~perf-raw_sample tools/perf/util/event.c
--- linux-2.6/tools/perf/util/event.c~perf-raw_sample	2009-12-07 14:52:56.000000000 +0900
+++ linux-2.6-hirofumi/tools/perf/util/event.c	2009-12-07 14:53:06.000000000 +0900
@@ -368,12 +368,8 @@ int event__parse_sample(event_t *event, 
 		array += 1 + data->callchain->nr;
 	}
 
-	if (type & PERF_SAMPLE_RAW) {
-		u32 *p = (u32 *)array;
-		data->raw_size = *p;
-		p++;
-		data->raw_data = p;
-	}
+	if (type & PERF_SAMPLE_RAW)
+		data->raw = (struct sample_raw_data *)array;
 
 	return 0;
 }
_

Re: [PATCH] perf_event: fix for processing raw event - fix

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

> Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> writes:
>
>> raw->size is not used, this patch just cleanup it
>
> Oops, I didn't notice those. Thanks.
>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
>> ---
>>  tools/perf/builtin-kmem.c  |   38 +++++++-----------
>>  tools/perf/builtin-sched.c |   94 +++++++++++++++++++------------------------
>>  2 files changed, 56 insertions(+), 76 deletions(-)
>>
>> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
>> index f84d7a3..7551a5f 100644
>> --- a/tools/perf/builtin-kmem.c
>> +++ b/tools/perf/builtin-kmem.c
>> @@ -57,11 +57,6 @@ static struct rb_root root_caller_sorted;
>>  static unsigned long total_requested, total_allocated;
>>  static unsigned long nr_allocs, nr_cross_allocs;
>>  
>> -struct raw_event_sample {
>> -	u32 size;
>> -	char data[0];
>> -};
>> -
>>  #define PATH_SYS_NODE	"/sys/devices/system/node"
>>  
>>  static void init_cpunode_map(void)
>> @@ -201,7 +196,7 @@ static void insert_caller_stat(unsigned long call_site,
>>  	}
>>  }
>>  
>> -static void process_alloc_event(struct raw_event_sample *raw,
>> +static void process_alloc_event(void *data,
>>  				struct event *event,
>>  				int cpu,
>>  				u64 timestamp __used,
>
> How about the following patch instead of playing with unsafe "void *"?
> With this, I guess it does type check, and filed handling functions can

s/filed handling/field handling of sample raw data/

> check "size" by passing "struct sample_raw_data" instead of "void *" in
> future.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[tip:perf/urgent] perf_event: Fix raw event processing

[tip-bot for Xiao Guangrong]

Commit-ID:  d8bd9e0aedabcb47887712497bc386a06ddcbd12
Gitweb:     http://git.kernel.org/tip/d8bd9e0aedabcb47887712497bc386a06ddcbd12
Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
AuthorDate: Mon, 7 Dec 2009 12:06:29 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 7 Dec 2009 06:26:24 +0100

perf_event: Fix raw event processing

We use 'data.raw_data' parameter to call process_raw_event(),
but data.raw_data buffer not include data size. it can make perf
tool crash.

This bug was introduced by commit 180f95e29a ("perf: Make common
SAMPLE_EVENT parser").

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Li Zefan <lizf@cn.fujitsu.com>
LKML-Reference: <4B1C7F45.5080105@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-kmem.c  |   11 ++++++++---
 tools/perf/builtin-sched.c |   11 ++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index f218990..f84d7a3 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -289,13 +289,17 @@ static void process_free_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, u32 size, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
+	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
+	raw = malloc_or_die(sizeof(*raw)+size);
+	raw->size = size;
+	memcpy(raw->data, data, size);
+
 	type = trace_parse_common_type(raw->data);
 	event = trace_find_event(type);
 
@@ -345,7 +349,8 @@ static int process_sample_event(event_t *event)
 
 	dump_printf(" ... thread: %s:%d\n", thread->comm, thread->pid);
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+			  data.time, thread);
 
 	return 0;
 }
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 7481ebd..4655e16 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1570,13 +1570,17 @@ process_sched_migrate_task_event(struct raw_event_sample *raw,
 }
 
 static void
-process_raw_event(event_t *raw_event __used, void *more_data,
+process_raw_event(event_t *raw_event __used, u32 size, void *data,
 		  int cpu, u64 timestamp, struct thread *thread)
 {
-	struct raw_event_sample *raw = more_data;
+	struct raw_event_sample *raw;
 	struct event *event;
 	int type;
 
+	raw = malloc_or_die(sizeof(*raw)+size);
+	raw->size = size;
+	memcpy(raw->data, data, size);
+
 	type = trace_parse_common_type(raw->data);
 	event = trace_find_event(type);
 
@@ -1629,7 +1633,8 @@ static int process_sample_event(event_t *event)
 	if (profile_cpu != -1 && profile_cpu != (int)data.cpu)
 		return 0;
 
-	process_raw_event(event, data.raw_data, data.cpu, data.time, thread);
+	process_raw_event(event, data.raw_size, data.raw_data, data.cpu,
+			  data.time, thread);
 
 	return 0;
 }

[tip:perf/urgent] perf/sched: Fix 'perf sched trace'

[tip-bot for Xiao Guangrong]

Commit-ID:  c0777c5aa835a97ccc77d82e55388940f0140a61
Gitweb:     http://git.kernel.org/tip/c0777c5aa835a97ccc77d82e55388940f0140a61
Author:     Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
AuthorDate: Mon, 7 Dec 2009 12:04:49 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 7 Dec 2009 06:26:22 +0100

perf/sched: Fix 'perf sched trace'

If we use 'perf sched trace', it will call symbol__init() again,
and can lead to a perf tool crash:

 [root@localhost perf]# ./perf sched trace
 *** glibc detected *** ./perf: free(): invalid next size (normal): 0x094c1898 ***
 ======= Backtrace: =========
 /lib/libc.so.6[0xb7602404]
 /lib/libc.so.6(cfree+0x96)[0xb76043b6]
 ./perf[0x80730fe]
 ./perf[0x8074c97]
 ./perf[0x805eb59]
 ./perf[0x80536fd]
 ./perf[0x804b618]
 ./perf[0x804bdc3]
 /lib/libc.so.6(__libc_start_main+0xe5)[0xb75a9735]
 ./perf[0x804af81]
 ======= Memory map: ========
 08048000-08158000 r-xp 00000000 fe:00 556831     /home/eric/....
 08158000-08168000 rw-p 0010f000 fe:00 556831     /home/eric/...
 08168000-085fe000 rw-p 00000000 00:00 0
 094ab000-094cc000 rw-p 00000000 00:00 0          [heap]

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
LKML-Reference: <4B1C7EE1.8030906@cn.fujitsu.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 tools/perf/builtin-sched.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 45c46c7..7481ebd 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -1888,13 +1888,18 @@ static int __cmd_record(int argc, const char **argv)
 
 int cmd_sched(int argc, const char **argv, const char *prefix __used)
 {
-	symbol__init(0);
-
 	argc = parse_options(argc, argv, sched_options, sched_usage,
 			     PARSE_OPT_STOP_AT_NON_OPTION);
 	if (!argc)
 		usage_with_options(sched_usage, sched_options);
 
+	/*
+	 * Aliased to 'perf trace' for now:
+	 */
+	if (!strcmp(argv[0], "trace"))
+		return cmd_trace(argc, argv, prefix);
+
+	symbol__init(0);
 	if (!strncmp(argv[0], "rec", 3)) {
 		return __cmd_record(argc, argv);
 	} else if (!strncmp(argv[0], "lat", 3)) {
@@ -1918,11 +1923,6 @@ int cmd_sched(int argc, const char **argv, const char *prefix __used)
 				usage_with_options(replay_usage, replay_options);
 		}
 		__cmd_replay();
-	} else if (!strcmp(argv[0], "trace")) {
-		/*
-		 * Aliased to 'perf trace' for now:
-		 */
-		return cmd_trace(argc, argv, prefix);
 	} else {
 		usage_with_options(sched_usage, sched_options);
 	}