From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 123298 invoked by alias); 29 Nov 2018 19:52:02 -0000 Mailing-List: contact libc-stable-help@sourceware.org; run by ezmlm Precedence: bulk List-Post: List-Help: List-Subscribe: List-Archive: Sender: libc-stable-owner@sourceware.org Received: (qmail 122957 invoked by uid 89); 29 Nov 2018 19:52:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.2 on sourceware.org X-Virus-Found: No 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=linear, H*r:sk:libc-st X-Spam-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 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 29 Nov 2018 19:51:58 +0000 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D9D9A3138BA5 for ; Thu, 29 Nov 2018 19:51:56 +0000 (UTC) Received: from greed.delorie.com (ovpn-121-122.rdu2.redhat.com [10.10.121.122]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9FD6519C7B for ; Thu, 29 Nov 2018 19:51:56 +0000 (UTC) Received: from greed.delorie.com.redhat.com (localhost [127.0.0.1]) by greed.delorie.com (8.14.7/8.14.7) with ESMTP id wATJptD6003169 for ; Thu, 29 Nov 2018 14:51:55 -0500 Date: Mon, 01 Jan 2018 00:00:00 -0000 Message-Id: From: DJ Delorie To: libc-stable@sourceware.org Subject: [2.28 COMMITTED V2] malloc: tcache double free check X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Thu, 29 Nov 2018 19:51:56 +0000 (UTC) X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00031.txt.bz2 Second try for this patch; this one commit contains two cherry-picks - the original patch and Florian's fix for it. Tested by building F29 RPMs, installing them, then building glibc on that system. * malloc/malloc.c (tcache_entry): Add key field. (tcache_put): Set it. (tcache_get): Likewise. (_int_free): Check for double free in tcache. * malloc/tst-tcfree1.c: New. * malloc/tst-tcfree2.c: New. * malloc/Makefile: Run the new tests. * manual/probes.texi: Document memory_tcache_double_free probe. * dlfcn/dlerror.c (check_free): Prevent double frees. (cherry picked from commit bcdaad21d4635931d1bd3b54a7894276925d081d) 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. (cherry picked from commit affec03b713c82c43a5b025dddc21bde3334f41e) diff --git a/ChangeLog b/ChangeLog index a997003664..bc62a59c2b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,23 @@ +2018-11-26 Florian Weimer + + [BZ #23907] + * malloc/malloc.c (_int_free): Validate tc_idx before checking for + double-frees. + + +2018-11-20 DJ Delorie + + * malloc/malloc.c (tcache_entry): Add key field. + (tcache_put): Set it. + (tcache_get): Likewise. + (_int_free): Check for double free in tcache. + * malloc/tst-tcfree1.c: New. + * malloc/tst-tcfree2.c: New. + * malloc/Makefile: Run the new tests. + * manual/probes.texi: Document memory_tcache_double_free probe. + + * dlfcn/dlerror.c (check_free): Prevent double frees. + 2018-11-27 Florian Weimer [BZ #23927] 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 47795601c8..dad0e73735 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,11 @@ tcache_put (mchunkptr chunk, size_t tc_idx) { tcache_entry *e = (tcache_entry *) chunk2mem (chunk); assert (tc_idx < TCACHE_MAX_BINS); + + /* Mark this chunk as "in the tcache" so the test in _int_free 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 +2933,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; } @@ -4165,13 +4173,33 @@ _int_free (mstate av, mchunkptr p, int have_lock) #if USE_TCACHE { size_t tc_idx = csize2tidx (size); - - if (tcache - && tc_idx < mp_.tcache_bins - && tcache->counts[tc_idx] < mp_.tcache_count) + if (tcache != NULL && tc_idx < mp_.tcache_bins) { - tcache_put (p, tc_idx); - return; + /* 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^ 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->counts[tc_idx] < mp_.tcache_count) + { + tcache_put (p, tc_idx); + return; + } } } #endif diff --git a/malloc/tst-tcfree1.c b/malloc/tst-tcfree1.c new file mode 100644 index 0000000000..bc29375ce7 --- /dev/null +++ b/malloc/tst-tcfree1.c @@ -0,0 +1,42 @@ +/* Test that malloc tcache catches double free. + 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) +{ + /* Do one allocation of any size that fits in tcache. */ + 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..17f06bacd4 --- /dev/null +++ b/malloc/tst-tcfree2.c @@ -0,0 +1,48 @@ +/* Test that malloc tcache catches double free. + 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; + + /* Allocate enough small chunks so that when we free them all, the tcache + is full, and the first one we freed is at the end of its linked list. */ +#define COUNT 20 + for (i=0; i diff --git a/manual/probes.texi b/manual/probes.texi index ab2a3102bb..0ea560ed78 100644 --- a/manual/probes.texi +++ b/manual/probes.texi @@ -243,6 +243,18 @@ This probe is triggered when the value of this tunable. @end deftp +@deftp Probe memory_tcache_double_free (void *@var{$arg1}, int @var{$arg2}) +This probe is triggered when @code{free} determines that the memory +being freed has probably already been freed, and resides in the +per-thread cache. Note that there is an extremely unlikely chance +that this probe will trigger due to random payload data remaining in +the allocated memory matching the key used to detect double frees. +This probe actually indicates that an expensive linear search of the +tcache, looking for a double free, has happened. Argument @var{$arg1} +is the memory location as passed to @code{free}, Argument @var{$arg2} +is the tcache bin it resides in. +@end deftp + @node Mathematical Function Probes @section Mathematical Function Probes