forked from rpms/glibc
134 lines
4.6 KiB
Diff
134 lines
4.6 KiB
Diff
From fc859c304898a5ec72e0ba5269ed136ed0ea10e1 Mon Sep 17 00:00:00 2001
|
|
From: Siddhesh Poyarekar <siddhesh@sourceware.org>
|
|
Date: Wed, 7 Jul 2021 23:02:46 +0530
|
|
Subject: Harden tcache double-free check
|
|
|
|
The tcache allocator layer uses the tcache pointer as a key to
|
|
identify a block that may be freed twice. Since this is in the
|
|
application data area, an attacker exploiting a use-after-free could
|
|
potentially get access to the entire tcache structure through this
|
|
key. A detailed write-up was provided by Awarau here:
|
|
|
|
https://awaraucom.wordpress.com/2020/07/19/house-of-io-remastered/
|
|
|
|
Replace this static pointer use for key checking with one that is
|
|
generated at malloc initialization. The first attempt is through
|
|
getrandom with a fallback to random_bits(), which is a simple
|
|
pseudo-random number generator based on the clock. The fallback ought
|
|
to be sufficient since the goal of the randomness is only to make the
|
|
key arbitrary enough that it is very unlikely to collide with user
|
|
data.
|
|
|
|
Co-authored-by: Eyal Itkin <eyalit@checkpoint.com>
|
|
|
|
[note: context for arena.c chunk #2 changed to accomodate missing
|
|
tagging support code - DJ]
|
|
|
|
diff -rup a/malloc/arena.c b/malloc/arena.c
|
|
--- a/malloc/arena.c 2022-09-16 01:09:02.003843024 -0400
|
|
+++ b/malloc/arena.c 2022-09-16 01:25:51.879994057 -0400
|
|
@@ -286,6 +286,10 @@ extern struct dl_open_hook *_dl_open_hoo
|
|
libc_hidden_proto (_dl_open_hook);
|
|
#endif
|
|
|
|
+#if USE_TCACHE
|
|
+static void tcache_key_initialize (void);
|
|
+#endif
|
|
+
|
|
static void
|
|
ptmalloc_init (void)
|
|
{
|
|
@@ -294,6 +298,10 @@ ptmalloc_init (void)
|
|
|
|
__malloc_initialized = 0;
|
|
|
|
+#if USE_TCACHE
|
|
+ tcache_key_initialize ();
|
|
+#endif
|
|
+
|
|
#ifdef SHARED
|
|
/* In case this libc copy is in a non-default namespace, never use brk.
|
|
Likewise if dlopened from statically linked program. */
|
|
diff -rup a/malloc/malloc.c b/malloc/malloc.c
|
|
--- a/malloc/malloc.c 2022-09-16 01:09:05.491977387 -0400
|
|
+++ b/malloc/malloc.c 2022-09-16 01:25:51.883994213 -0400
|
|
@@ -247,6 +247,10 @@
|
|
/* For SINGLE_THREAD_P. */
|
|
#include <sysdep-cancel.h>
|
|
|
|
+/* For tcache double-free check. */
|
|
+#include <random-bits.h>
|
|
+#include <sys/random.h>
|
|
+
|
|
/*
|
|
Debugging:
|
|
|
|
@@ -2924,7 +2928,7 @@ typedef struct tcache_entry
|
|
{
|
|
struct tcache_entry *next;
|
|
/* This field exists to detect double frees. */
|
|
- struct tcache_perthread_struct *key;
|
|
+ uintptr_t key;
|
|
} tcache_entry;
|
|
|
|
/* There is one of these for each thread, which contains the
|
|
@@ -2941,6 +2945,31 @@ typedef struct tcache_perthread_struct
|
|
static __thread bool tcache_shutting_down = false;
|
|
static __thread tcache_perthread_struct *tcache = NULL;
|
|
|
|
+/* Process-wide key to try and catch a double-free in the same thread. */
|
|
+static uintptr_t tcache_key;
|
|
+
|
|
+/* The value of tcache_key does not really have to be a cryptographically
|
|
+ secure random number. It only needs to be arbitrary enough so that it does
|
|
+ not collide with values present in applications. If a collision does happen
|
|
+ consistently enough, it could cause a degradation in performance since the
|
|
+ entire list is checked to check if the block indeed has been freed the
|
|
+ second time. The odds of this happening are exceedingly low though, about 1
|
|
+ in 2^wordsize. There is probably a higher chance of the performance
|
|
+ degradation being due to a double free where the first free happened in a
|
|
+ different thread; that's a case this check does not cover. */
|
|
+static void
|
|
+tcache_key_initialize (void)
|
|
+{
|
|
+ if (__getrandom (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK)
|
|
+ != sizeof (tcache_key))
|
|
+ {
|
|
+ tcache_key = random_bits ();
|
|
+#if __WORDSIZE == 64
|
|
+ tcache_key = (tcache_key << 32) | random_bits ();
|
|
+#endif
|
|
+ }
|
|
+}
|
|
+
|
|
/* Caller must ensure that we know tc_idx is valid and there's room
|
|
for more chunks. */
|
|
static __always_inline void
|
|
@@ -2950,7 +2979,7 @@ tcache_put (mchunkptr chunk, size_t tc_i
|
|
|
|
/* Mark this chunk as "in the tcache" so the test in _int_free will
|
|
detect a double free. */
|
|
- e->key = tcache;
|
|
+ e->key = tcache_key;
|
|
|
|
e->next = PROTECT_PTR (&e->next, tcache->entries[tc_idx]);
|
|
tcache->entries[tc_idx] = e;
|
|
@@ -2967,7 +2996,7 @@ tcache_get (size_t tc_idx)
|
|
malloc_printerr ("malloc(): unaligned tcache chunk detected");
|
|
tcache->entries[tc_idx] = REVEAL_PTR (e->next);
|
|
--(tcache->counts[tc_idx]);
|
|
- e->key = NULL;
|
|
+ e->key = 0;
|
|
return (void *) e;
|
|
}
|
|
|
|
@@ -4231,7 +4260,7 @@ _int_free (mstate av, mchunkptr p, int h
|
|
trust it (it also matches random payload data at a 1 in
|
|
2^<size_t> chance), so verify it's not an unlikely
|
|
coincidence before aborting. */
|
|
- if (__glibc_unlikely (e->key == tcache))
|
|
+ if (__glibc_unlikely (e->key == tcache_key))
|
|
{
|
|
tcache_entry *tmp;
|
|
size_t cnt = 0;
|