This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch] tcache double free check


This patch uses a spare field in the tcache chunk overlay to store a
key, to detect if something is (or at least should be) in the tcache,
without having to iterate through the tcache.  While not perfect, it
allows us to detect many double free situations.  Not covered - if the
second free is by a different thread, because the second thread
wouldn't have visibility into the first thread's tcache (duh).

Includes test cases.  Detected one double free in elf/tst-leaks1,
which I also include a fix for.

	* malloc/malloc.c (tcache_entry): Add key field.
	(tcache_put): Check for double free in tcache.
	(tcache_get): Likewise.
	(_int_free): Likewise.
	* malloc/tst-tcfree1.c: New.
	* malloc/tst-tcfree2.c: New.
	* malloc/Makefile: Run the new tests.

	* dlfcn/dlerror.c (check_free): Prevent double frees.

diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c
index 33574faab6..96bf925333 100644
--- a/dlfcn/dlerror.c
+++ b/dlfcn/dlerror.c
@@ -198,7 +198,10 @@ check_free (struct dl_action_result *rec)
       Dl_info info;
       if (_dl_addr (check_free, &info, &map, NULL) != 0 && map->l_ns == 0)
 #endif
-	free ((char *) rec->errstring);
+	{
+	  free ((char *) rec->errstring);
+	  rec->errstring = NULL;
+	}
     }
 }
 
diff --git a/malloc/Makefile b/malloc/Makefile
index 7d54bad866..e6dfbfc14c 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -38,6 +38,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-malloc_info \
 	 tst-malloc-too-large \
 	 tst-malloc-stats-cancellation \
+	 tst-tcfree1 tst-tcfree2 \
 
 tests-static := \
 	 tst-interpose-static-nothread \
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 67cdfd0ad2..beaab29a05 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -2888,6 +2888,8 @@ mremap_chunk (mchunkptr p, size_t new_size)
 typedef struct tcache_entry
 {
   struct tcache_entry *next;
+  /* This field exists to detect double frees.  */
+  struct tcache_perthread_struct *key;
 } tcache_entry;
 
 /* There is one of these for each thread, which contains the
@@ -2911,6 +2913,27 @@ tcache_put (mchunkptr chunk, size_t tc_idx)
 {
   tcache_entry *e = (tcache_entry *) chunk2mem (chunk);
   assert (tc_idx < TCACHE_MAX_BINS);
+
+  /* This test succeeds on double free.  However, we don't 100% trust
+     it, so verify it's not an unlikely coincidence before
+     aborting.  */
+  if (__glibc_unlikely (e->key == tcache))
+    {
+      tcache_entry *tmp;
+      for (tmp = tcache->entries[tc_idx];
+	   tmp;
+	   tmp = tmp->next)
+	{
+	if (tmp == e)
+	  malloc_printerr ("free(): double free detected in tcache");
+	}
+      /* If we get here, it was a coincidence.  We've wasted a few
+	 cycles, but don't abort.  */
+    }
+  /* Now we mark this chunk as "in the tcache" so the above test will
+     detect a double free.  */
+  e->key = tcache;
+
   e->next = tcache->entries[tc_idx];
   tcache->entries[tc_idx] = e;
   ++(tcache->counts[tc_idx]);
@@ -2926,6 +2949,7 @@ tcache_get (size_t tc_idx)
   assert (tcache->entries[tc_idx] > 0);
   tcache->entries[tc_idx] = e->next;
   --(tcache->counts[tc_idx]);
+  e->key = NULL;
   return (void *) e;
 }
 
@@ -4173,6 +4197,21 @@ _int_free (mstate av, mchunkptr p, int have_lock)
 	tcache_put (p, tc_idx);
 	return;
       }
+    else
+      {
+	/* Check to see if it's already in the tcache.  */
+	tcache_entry *e = (tcache_entry *) chunk2mem (p);
+	/* Copy of test from tcache_put.  */
+	if (__glibc_unlikely (e->key == tcache && tcache))
+	  {
+	    tcache_entry *tmp;
+	    for (tmp = tcache->entries[tc_idx];
+		 tmp;
+		 tmp = tmp->next)
+	      if (tmp == e)
+		malloc_printerr ("free(): double free detected in tcache 2");
+	  }
+      }
   }
 #endif
 
diff --git a/malloc/tst-tcfree1.c b/malloc/tst-tcfree1.c
new file mode 100644
index 0000000000..b1258f403f
--- /dev/null
+++ b/malloc/tst-tcfree1.c
@@ -0,0 +1,40 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <error.h>
+#include <limits.h>
+#include <malloc.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/signal.h>
+
+static int
+do_test (void)
+{
+  char * volatile x = malloc (32);
+
+  free (x); // puts in tcache
+  free (x); // should abort
+
+  printf("FAIL: tcache double free not detected\n");
+  return 1;
+}
+
+#define TEST_FUNCTION do_test
+#define EXPECTED_SIGNAL SIGABRT
+#include <support/test-driver.c>
diff --git a/malloc/tst-tcfree2.c b/malloc/tst-tcfree2.c
new file mode 100644
index 0000000000..2acc3036b5
--- /dev/null
+++ b/malloc/tst-tcfree2.c
@@ -0,0 +1,45 @@
+/* Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <error.h>
+#include <limits.h>
+#include <malloc.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <sys/signal.h>
+
+static int
+do_test (void)
+{
+  char * volatile ptrs[20];
+  int i;
+
+#define COUNT 20
+  for (i=0; i<COUNT; i++)
+    ptrs[i] = malloc (20);
+  for (i=0; i<COUNT; i++)
+    free (ptrs[i]);
+  free (ptrs[0]);
+
+  printf("FAIL: tcache double free\n");
+  return 1;
+}
+
+#define TEST_FUNCTION do_test
+#define EXPECTED_SIGNAL SIGABRT
+#include <support/test-driver.c>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]