malloc: tcache: Validate tc_idx before checking for double-frees [BZ #23907]

The previous check could read beyond the end of the tcache entry
array.  If the e->key == tcache cookie check happened to pass, this
would result in crashes.

diff --git a/ChangeLog b/ChangeLog
index 77fb773aea43b7588407429d6898b651cc6832bf..84ddd68d7d209b6886053e3c4107dc0752a501d6 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2018-11-26  Florian Weimer  <fweimer@redhat.com>
+
+       [BZ #23907]
+       * malloc/malloc.c (_int_free): Validate tc_idx before checking for
+       double-frees.
+
 2018-11-26  Rafael Ávila de Espíndola  <rafael@espindo.la>
 
        [BZ #19767]

diff --git a/malloc/malloc.c b/malloc/malloc.c
index f730d7a2ee496d365bf3546298b9d19b8bddc0d0..c9b2c6e320e5eaa3d91cde95aa61f289e407418e 100644 (file)
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -4225,33 +4225,33 @@ _int_free (mstate av, mchunkptr p, int have_lock)
 #if USE_TCACHE
   {
     size_t tc_idx = csize2tidx (size);
-
-    /* Check to see if it's already in the tcache.  */
-    tcache_entry *e = (tcache_entry *) chunk2mem (p);
-
-    /* This test succeeds on double free.  However, we don't 100%
-       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 && tcache))
+    if (tcache != NULL && tc_idx < mp_.tcache_bins)
       {
-       tcache_entry *tmp;
-       LIBC_PROBE (memory_tcache_double_free, 2, e, tc_idx);
-       for (tmp = tcache->entries[tc_idx];
-            tmp;
-            tmp = tmp->next)
-         if (tmp == e)
-           malloc_printerr ("free(): double free detected in tcache 2");
-       /* If we get here, it was a coincidence.  We've wasted a few
-          cycles, but don't abort.  */
-      }
+       /* Check to see if it's already in the tcache.  */
+       tcache_entry *e = (tcache_entry *) chunk2mem (p);
+
+       /* This test succeeds on double free.  However, we don't 100%
+          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))
+         {
+           tcache_entry *tmp;
+           LIBC_PROBE (memory_tcache_double_free, 2, e, tc_idx);
+           for (tmp = tcache->entries[tc_idx];
+                tmp;
+                tmp = tmp->next)
+             if (tmp == e)
+               malloc_printerr ("free(): double free detected in tcache 2");
+           /* If we get here, it was a coincidence.  We've wasted a
+              few cycles, but don't abort.  */
+         }
 
-    if (tcache
-       && tc_idx < mp_.tcache_bins
-       && tcache->counts[tc_idx] < mp_.tcache_count)
-      {
-       tcache_put (p, tc_idx);
-       return;
+       if (tcache->counts[tc_idx] < mp_.tcache_count)
+         {
+           tcache_put (p, tc_idx);
+           return;
+         }
       }
   }
 #endif