From patchwork Wed Nov 7 03:24:56 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: DJ Delorie X-Patchwork-Id: 30052 Received: (qmail 33950 invoked by alias); 7 Nov 2018 03:25:05 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 33881 invoked by uid 89); 7 Nov 2018 03:25:03 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Prevent X-HELO: mx1.redhat.com Date: Tue, 06 Nov 2018 22:24:56 -0500 Message-Id: From: DJ Delorie To: libc-alpha@sourceware.org Subject: [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 + . */ + +#include +#include +#include +#include +#include +#include +#include + +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 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 + . */ + +#include +#include +#include +#include +#include +#include +#include + +static int +do_test (void) +{ + char * volatile ptrs[20]; + int i; + +#define COUNT 20 + for (i=0; i