linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephane Eranian <eranian@google.com>
To: linux-kernel@vger.kernel.org
Cc: acme@redhat.com, peterz@infradead.org, mingo@elte.hu, jolsa@redhat.com
Subject: [PATCH] perf ordered_events: fix crash in free_dup_event()
Date: Mon,  6 Aug 2018 18:23:35 -0700	[thread overview]
Message-ID: <1533605015-19514-1-git-send-email-eranian@google.com> (raw)

Depending on memory allocations, it was possible to get a SEGFAULT in
free_dup_event() because the event pointer was bogus:

perf[1354]: segfault at ffffffff00000006 ip 00000000004b7fc7

Initially, I thought it was some double free. But it turns out
it looked more like a buffer overrun. Adding padding before
the union perf_event field in struct ordered_event avoided the
problem. But it was not obvious where this could be coming from
given the accesses to the struct, i.e.,  no internal array.

Then, it struck me that the following was bogus in __dup_event():

 __dup_event(struct ordered_events *oe, union perf_event *event)
{
   ...
   union perf_event *new_event;
   new_event = memdup(event, event->header.size);
   ...
}

The problem here is that header.size <= sizeof(*new_event). The code
was trying to copy only what was necessary, but then, the allocation
was also only partial. In other words if the event was not the largest
possible for the union, it would not be fully backed by memory, likely
causing troubles.

This patch fixes the problem by passing the size of the union and not
that of the actual event.

Signed-off-by: Stephane Eranian <eranian@google.com>

---
 tools/perf/util/ordered-events.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/ordered-events.c b/tools/perf/util/ordered-events.c
index bad9e0296e9a..a90dbe5df019 100644
--- a/tools/perf/util/ordered-events.c
+++ b/tools/perf/util/ordered-events.c
@@ -66,9 +66,9 @@ static union perf_event *__dup_event(struct ordered_events *oe,
 	union perf_event *new_event = NULL;
 
 	if (oe->cur_alloc_size < oe->max_alloc_size) {
-		new_event = memdup(event, event->header.size);
+		new_event = memdup(event, sizeof(*event));
 		if (new_event)
-			oe->cur_alloc_size += event->header.size;
+			oe->cur_alloc_size += sizeof(*event);
 	}
 
 	return new_event;
-- 
2.18.0.597.ga71716f1ad-goog


             reply	other threads:[~2018-08-07  1:24 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-07  1:23 Stephane Eranian [this message]
2018-08-07  7:20 ` [PATCH] perf ordered_events: fix crash in free_dup_event() Jiri Olsa
2018-08-07  8:16   ` Stephane Eranian
2018-08-07  8:50     ` Jiri Olsa
2018-08-07 19:11       ` Stephane Eranian
2018-08-08  8:23         ` Jiri Olsa
2018-08-08 21:47           ` Stephane Eranian
2018-08-09  7:39             ` Jiri Olsa

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=1533605015-19514-1-git-send-email-eranian@google.com \
    --to=eranian@google.com \
    --cc=acme@redhat.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).