[PATCH] Update tcache double-free check

[Eyal Itkin via Libc-alpha <libc-alpha@sourceware.org>]

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Hello,

As we discussed, I've attached here the patch for updating the
double-free check in the tcache. The patch passes all of malloc's
existing tests (including the double free tests), and it was tested to
work as expected both with and without entropy.

Once again, I apologize for sending the patch as an attachment instead
of inlined in the body of the mail itself (same Gmail issues as
before).

I am aware that there might be whitespace issues with the patch,
please feel free to fix them on your end if possible.
Thanks,
Eyal Itkin.

[-- Attachment #2: 0001-Update-tcache-double-free-check.patch --]
[-- Type: application/octet-stream, Size: 3038 bytes --]

From 32eee265a6574365246b9d89c68baed1e5aab53e Mon Sep 17 00:00:00 2001
From: Eyal Itkin <eyalit@checkpoint.com>
Date: Fri, 24 Jul 2020 16:09:33 +0300
Subject: [PATCH] Update tcache double-free check

Update the value used for the tcache entry->key when checking for
double free operations. Use a random value by default, and ~tcache as
a backup value if there isn't enough entropy / entropy isn't available.

Original key value was "tcache" which may lead to security issues in
code with use-after-free vulnerabilities ("House of Io" exploit). The
new key is no longer a valid pointer to a critical meta-data struct.
---
 malloc/malloc.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/malloc/malloc.c b/malloc/malloc.c
index ee87ddbbf9..37d6d62a6d 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -247,6 +247,9 @@
 /* For SINGLE_THREAD_P.  */
 #include <sysdep-cancel.h>
 
+/* For tcache double-free checks.  */
+#include <sys/random.h>
+
 /*
   Debugging:
 
@@ -2910,7 +2913,7 @@ typedef struct tcache_entry
 {
   struct tcache_entry *next;
   /* This field exists to detect double frees.  */
-  struct tcache_perthread_struct *key;
+  size_t key;
 } tcache_entry;
 
 /* There is one of these for each thread, which contains the
@@ -2926,6 +2929,7 @@ typedef struct tcache_perthread_struct
 
 static __thread bool tcache_shutting_down = false;
 static __thread tcache_perthread_struct *tcache = NULL;
+static __thread size_t tcache_key = 0;
 
 /* Caller must ensure that we know tc_idx is valid and there's room
    for more chunks.  */
@@ -2936,7 +2940,7 @@ tcache_put (mchunkptr chunk, size_t tc_idx)
 
   /* 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;
@@ -2953,7 +2957,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;
 }
 
@@ -3019,6 +3023,12 @@ tcache_init(void)
     {
       tcache = (tcache_perthread_struct *) victim;
       memset (tcache, 0, sizeof (tcache_perthread_struct));
+
+      /* Attempt to get a random key for the double-free checks.  */
+      int res = getrandom (&tcache_key, sizeof(tcache_key), GRND_NONBLOCK);
+      /* If failed, use the agreed alternative: ~tcache.  */
+      if (res != sizeof(tcache_key))
+        tcache_key = ~((size_t) tcache);
     }
 
 }
@@ -4218,7 +4228,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
 	   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;
 	    LIBC_PROBE (memory_tcache_double_free, 2, e, tc_idx);
-- 
2.17.1