linux-kernel - [PATCH] perf ordered_events: fix crash in free_dup

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-next>] [day] [month] [year] [list]

Message-Id: <1533605015-19514-1-git-send-email-eranian@google.com>
Date:   Mon,  6 Aug 2018 18:23:35 -0700
From:   Stephane Eranian <eranian@...gle.com>
To:     linux-kernel@...r.kernel.org
Cc:     acme@...hat.com, peterz@...radead.org, mingo@...e.hu,
        jolsa@...hat.com
Subject: [PATCH] perf ordered_events: fix crash in free_dup_event()

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