From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25728 invoked by alias); 22 Nov 2018 21:13:58 -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 25717 invoked by uid 89); 22 Nov 2018 21:13:58 -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=-25.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,KAM_LAZY_DOMAIN_SECURITY,KAM_SHORT,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=rec X-Spam-Status: No, score=-25.1 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,KAM_LAZY_DOMAIN_SECURITY,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, 22 Nov 2018 21:13:56 +0000 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C850630001EC for ; Thu, 22 Nov 2018 21:13:54 +0000 (UTC) Received: from oldenburg.str.redhat.com (ovpn-116-170.ams2.redhat.com [10.36.116.170]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DC260100034B; Thu, 22 Nov 2018 21:13:53 +0000 (UTC) From: Florian Weimer To: DJ Delorie Cc: libc-stable@sourceware.org Subject: Re: [2.28 COMMITTED] malloc: tcache double free check References: Date: Mon, 01 Jan 2018 00:00:00 -0000 In-Reply-To: (DJ Delorie's message of "Tue, 20 Nov 2018 14:19:40 -0500") Message-ID: <87zhu0x1bp.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.45]); Thu, 22 Nov 2018 21:13:54 +0000 (UTC) X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg00027.txt.bz2 * DJ Delorie: > This adds a test that exists in the non-tcache malloc, hence > technically it fixes a regression - simple double frees were detected > before, tcache bypassed that test, so needs its own test. > > (cherry picked from commit bcdaad21d4635931d1bd3b54a7894276925d081d) The =E2=80=9Ccherry picked from=E2=80=9D line did not make it into your act= ual commit. I have reverted this with the patch below due to bug 23907. Thanks, Florian Subject: Revert "malloc: tcache double free check" [BZ #23907] This reverts commit 481a6cf0c24f02f251d7cd0b776c12d00e6b144f, the backport of commit bcdaad21d4635931d1bd3b54a7894276925d081d on the master branch. --- ChangeLog | 12 ------------ dlfcn/dlerror.c | 5 +---- malloc/Makefile | 1 - malloc/malloc.c | 28 ---------------------------- malloc/tst-tcfree1.c | 42 ------------------------------------------ malloc/tst-tcfree2.c | 48 ------------------------------------------------ manual/probes.texi | 12 ------------ 7 files changed, 1 insertion(+), 147 deletions(-) delete mode 100644 malloc/tst-tcfree1.c delete mode 100644 malloc/tst-tcfree2.c diff --git a/ChangeLog b/ChangeLog index 1ef4b4abe0..8c92ee7764 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,15 +1,3 @@ -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-19 Florian Weimer =20 support: Print timestamps in timeout handler. diff --git a/dlfcn/dlerror.c b/dlfcn/dlerror.c index 96bf925333..33574faab6 100644 --- a/dlfcn/dlerror.c +++ b/dlfcn/dlerror.c @@ -198,10 +198,7 @@ check_free (struct dl_action_result *rec) Dl_info info; if (_dl_addr (check_free, &info, &map, NULL) !=3D 0 && map->l_ns =3D= =3D 0) #endif - { - free ((char *) rec->errstring); - rec->errstring =3D NULL; - } + free ((char *) rec->errstring); } } =20 diff --git a/malloc/Makefile b/malloc/Makefile index e6dfbfc14c..7d54bad866 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -38,7 +38,6 @@ tests :=3D mallocbug tst-malloc tst-valloc tst-calloc tst= -obstack \ tst-malloc_info \ tst-malloc-too-large \ tst-malloc-stats-cancellation \ - tst-tcfree1 tst-tcfree2 \ =20 tests-static :=3D \ tst-interpose-static-nothread \ diff --git a/malloc/malloc.c b/malloc/malloc.c index 6be2573868..47795601c8 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -2888,8 +2888,6 @@ 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; =20 /* There is one of these for each thread, which contains the @@ -2913,11 +2911,6 @@ tcache_put (mchunkptr chunk, size_t tc_idx) { tcache_entry *e =3D (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 =3D tcache; - e->next =3D tcache->entries[tc_idx]; tcache->entries[tc_idx] =3D e; ++(tcache->counts[tc_idx]); @@ -2933,7 +2926,6 @@ tcache_get (size_t tc_idx) assert (tcache->entries[tc_idx] > 0); tcache->entries[tc_idx] =3D e->next; --(tcache->counts[tc_idx]); - e->key =3D NULL; return (void *) e; } =20 @@ -4174,26 +4166,6 @@ _int_free (mstate av, mchunkptr p, int have_lock) { size_t tc_idx =3D csize2tidx (size); =20 - /* Check to see if it's already in the tcache. */ - tcache_entry *e =3D (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 =3D=3D tcache && tcache)) - { - tcache_entry *tmp; - LIBC_PROBE (memory_tcache_double_free, 2, e, tc_idx); - for (tmp =3D tcache->entries[tc_idx]; - tmp; - tmp =3D tmp->next) - if (tmp =3D=3D 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) diff --git a/malloc/tst-tcfree1.c b/malloc/tst-tcfree1.c deleted file mode 100644 index bc29375ce7..0000000000 --- a/malloc/tst-tcfree1.c +++ /dev/null @@ -1,42 +0,0 @@ -/* 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 =3D 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 deleted file mode 100644 index 17f06bacd4..0000000000 --- a/malloc/tst-tcfree2.c +++ /dev/null @@ -1,48 +0,0 @@ -/* 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=3D0; i diff --git a/manual/probes.texi b/manual/probes.texi index 0ea560ed78..ab2a3102bb 100644 --- a/manual/probes.texi +++ b/manual/probes.texi @@ -243,18 +243,6 @@ This probe is triggered when the value of this tunable. @end deftp =20 -@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 =20 --=20 2.14.5