All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Wang Nan <wangnan0@huawei.com>,
	Ingo Molnar <mingo@redhat.com>,
	Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>,
	Namhyung Kim <namhyung@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Zefan Li <lizefan@huawei.com>,
	pi3orama@163.com, Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 2/8] perf tools: Fix a problem when opening old perf.data with different byte order
Date: Wed, 17 Jun 2015 18:22:29 -0300	[thread overview]
Message-ID: <1434576155-30038-3-git-send-email-acme@kernel.org> (raw)
In-Reply-To: <1434576155-30038-1-git-send-email-acme@kernel.org>

From: Wang Nan <wangnan0@huawei.com>

Following error occurs when trying to use 'perf report' on x86_64 to
cross analysis a perf.data generated by an old perf on a big-endian
machine:

 # perf report
 *** Error in `/home/w00229757/perf': free(): invalid next size (fast): 0x00000000032c99f0 ***
 ======= Backtrace: =========
 /lib64/libc.so.6(+0x6eeef)[0x7ff6ff7e2eef]
 /lib64/libc.so.6(+0x78cae)[0x7ff6ff7eccae]
 /lib64/libc.so.6(+0x79987)[0x7ff6ff7ed987]
 /path/to/perf[0x4ac734]
 /path/to/perf[0x4ac829]
 /path/to/perf(perf_header__process_sections+0x129)[0x4ad2c9]
 /path/to/perf(perf_session__read_header+0x2e1)[0x4ad9e1]
 /path/to/perf(perf_session__new+0x168)[0x4bd458]
 /path/to/perf(cmd_report+0xfa0)[0x43eb70]
 /path/to/perf[0x47adc3]
 /path/to/perf(main+0x5f6)[0x42fd06]
 /lib64/libc.so.6(__libc_start_main+0xf5)[0x7ff6ff795bd5]
 /path/to/perf[0x42fe35]
 ======= Memory map: ========
 [SNIP]

The bug is in perf_event__attr_swap(). It swaps all fields in 'struct
perf_event_attr' without checking whether the swapped field exist or
not. In addition, in read_event_desc() allocs memory for attr according
to size read from perf.data.

Therefore, if the perf.data is collected by an old perf (without
aux_watermark, for example), when perf_event__attr_swap() swaping
attr->aux_watermark it destroy malloc's metadata.

This patch introduces boundary checking in perf_event__attr_swap(). It
adds macros bswap_field_64 and bswap_field_32 into
perf_event__attr_swap() to make it only swap exist fields.

Signed-off-by: Wang Nan <wangnan0@huawei.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Zefan Li <lizefan@huawei.com>
Cc: pi3orama@163.com
Link: http://lkml.kernel.org/r/1434534999-85347-1-git-send-email-wangnan0@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 50 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index f31e024ddf7d..e1cd17c2afab 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -517,20 +517,42 @@ void perf_event__attr_swap(struct perf_event_attr *attr)
 {
 	attr->type		= bswap_32(attr->type);
 	attr->size		= bswap_32(attr->size);
-	attr->config		= bswap_64(attr->config);
-	attr->sample_period	= bswap_64(attr->sample_period);
-	attr->sample_type	= bswap_64(attr->sample_type);
-	attr->read_format	= bswap_64(attr->read_format);
-	attr->wakeup_events	= bswap_32(attr->wakeup_events);
-	attr->bp_type		= bswap_32(attr->bp_type);
-	attr->bp_addr		= bswap_64(attr->bp_addr);
-	attr->bp_len		= bswap_64(attr->bp_len);
-	attr->branch_sample_type = bswap_64(attr->branch_sample_type);
-	attr->sample_regs_user	 = bswap_64(attr->sample_regs_user);
-	attr->sample_stack_user  = bswap_32(attr->sample_stack_user);
-	attr->aux_watermark	 = bswap_32(attr->aux_watermark);
-
-	swap_bitfield((u8 *) (&attr->read_format + 1), sizeof(u64));
+
+#define bswap_safe(f, n) 					\
+	(attr->size > (offsetof(struct perf_event_attr, f) + 	\
+		       sizeof(attr->f) * (n)))
+#define bswap_field(f, sz) 			\
+do { 						\
+	if (bswap_safe(f, 0))			\
+		attr->f = bswap_##sz(attr->f);	\
+} while(0)
+#define bswap_field_32(f) bswap_field(f, 32)
+#define bswap_field_64(f) bswap_field(f, 64)
+
+	bswap_field_64(config);
+	bswap_field_64(sample_period);
+	bswap_field_64(sample_type);
+	bswap_field_64(read_format);
+	bswap_field_32(wakeup_events);
+	bswap_field_32(bp_type);
+	bswap_field_64(bp_addr);
+	bswap_field_64(bp_len);
+	bswap_field_64(branch_sample_type);
+	bswap_field_64(sample_regs_user);
+	bswap_field_32(sample_stack_user);
+	bswap_field_32(aux_watermark);
+
+	/*
+	 * After read_format are bitfields. Check read_format because
+	 * we are unable to use offsetof on bitfield.
+	 */
+	if (bswap_safe(read_format, 1))
+		swap_bitfield((u8 *) (&attr->read_format + 1),
+			      sizeof(u64));
+#undef bswap_field_64
+#undef bswap_field_32
+#undef bswap_field
+#undef bswap_safe
 }
 
 static void perf_event__hdr_attr_swap(union perf_event *event,
-- 
2.1.0


  parent reply	other threads:[~2015-06-17 21:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17 21:22 [GIT PULL 0/8] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 1/8] perf tools: Ignore .config-detected in .gitignore Arnaldo Carvalho de Melo
2015-06-17 21:22 ` Arnaldo Carvalho de Melo [this message]
2015-06-17 21:22 ` [PATCH 3/8] perf tools: Move libtraceevent dynamic list to separated LDFLAGS variable Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 4/8] perf probe: Show usage even if the last event is skipped Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 5/8] perf probe: Speed up perf probe --list by caching debuginfo Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 6/8] perf trace: Fix race condition at the end of started workloads Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 7/8] perf evlist: Add toggle_enable() method Arnaldo Carvalho de Melo
2015-06-17 21:22 ` [PATCH 8/8] perf top: Allow disabling/enabling events dynamicly Arnaldo Carvalho de Melo
2015-06-18  7:40 ` [GIT PULL 0/8] perf/core improvements and fixes Ingo Molnar
2015-06-18 20:18   ` [RFC] hotkey for disabling/enabling events in 'perf top' TUI was " Arnaldo Carvalho de Melo
2015-06-18 20:58     ` Ingo Molnar
2015-06-18 21:39       ` Arnaldo Carvalho de Melo
2015-06-19  6:27         ` Ingo Molnar
2015-06-19 20:07           ` Arnaldo Carvalho de Melo
2015-06-19 23:05             ` Ingo Molnar
2015-06-22 14:52               ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1434576155-30038-3-git-send-email-acme@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lizefan@huawei.com \
    --cc=masami.hiramatsu.pt@hitachi.com \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=pi3orama@163.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.