Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
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
Message-Id: <1533605015-19514-1-git-send-email-eranian@google.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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