From patchwork Mon Nov 12 20:38:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Thomas X-Patchwork-Id: 996642 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-97182-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=thomii.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="UwHa3TTk"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42v2fX0MJpz9s7W for ; Tue, 13 Nov 2018 07:38:59 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type:content-transfer-encoding; q=dns; s=default; b=Jfj P+ygo8vL7y0KXfC3H3lXyvHAfcmqlL7gDIkgjWwN95bcCdKJAHvkmKys6NPqodd6 X1Y+jA7R5gaoNEOvLbg0UaJQMvIodTCz4bFCb1JXXlTNtIeVxsv7JQOKc9yJXSmw GKqTGtHDp3Tw7bfCw8pvtBbHVGmFoRMX+Wx2ym10= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:from:to:subject:message-id:mime-version :content-type:content-transfer-encoding; s=default; bh=9EH4+5co8 SzBo3zKpnswq5BJcGM=; b=UwHa3TTk0i/sk3uJ4vSbRbehwouYPCW5BJ/cUuuD0 4qa2IAYNkYmpLiBE5YfeZqKNlJcAWI+F8km+oOLDb7oNeo+8zfJUX52lO6rxtMRp aJIIPCpWV/HIcA/ViXCyYPTrj+R5rURfWPlU6F5hcTxOY1mAoiJivQoGuQ8XCFLL JM= Received: (qmail 84588 invoked by alias); 12 Nov 2018 20:38:54 -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 84572 invoked by uid 89); 12 Nov 2018 20:38:52 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, KAM_SHORT autolearn=ham version=3.3.2 spammy=chosen, 1112, H*RU:sk:server2, Hx-spam-relays-external:sk:server2 X-HELO: server2k12.thomii.com Date: Mon, 12 Nov 2018 15:38:09 -0500 From: Mike Thomas To: Subject: [PATCHv2/22885] Detect altered malloc chunks Message-ID: <5be9e4b1.f8ONiDEbTYBFjqwT%michael@thomii.com> User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Fixes bug 22885 v2 Addressed all comments given in v1 of submission. Now using email address that matches email address in copyright assignment. Includes test cases. Tested with no regressions. * malloc/Makefile: Run the new tests. * malloc/malloc.c (tcache_get): Add consistency check. (_int_malloc): Likewise. * malloc/tst-malloc-smallbins-chunksize.c: New. * malloc/tst-malloc-smallbins2tcache-chunksize.c: New. * malloc/tst-malloc-tcache-chunksize.c: New. diff --git a/malloc/Makefile b/malloc/Makefile index 7d54bad866..1bb3543a7a 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -38,6 +38,9 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-malloc_info \ tst-malloc-too-large \ tst-malloc-stats-cancellation \ + tst-malloc-tcache-chunksize \ + tst-malloc-smallbins-chunksize \ + tst-malloc-smallbins2tcache-chunksize \ tests-static := \ tst-interpose-static-nothread \ @@ -194,6 +197,8 @@ tst-malloc-usable-ENV = MALLOC_CHECK_=3 tst-malloc-usable-static-ENV = $(tst-malloc-usable-ENV) tst-malloc-usable-tunables-ENV = GLIBC_TUNABLES=glibc.malloc.check=3 tst-malloc-usable-static-tunables-ENV = $(tst-malloc-usable-tunables-ENV) +tst-malloc-smallbins-chunksize-ENV = GLIBC_TUNABLES=glibc.malloc.tcache_count=0 +tst-malloc-smallbins2tcache-chunksize-ENV = GLIBC_TUNABLES=glibc.malloc.tcache_count=4 ifeq ($(experimental-malloc),yes) CPPFLAGS-malloc.c += -DUSE_TCACHE=1 diff --git a/malloc/malloc.c b/malloc/malloc.c index 67cdfd0ad2..ac59f6f34b 100644 --- a/malloc/malloc.c +++ b/malloc/malloc.c @@ -2924,6 +2924,17 @@ tcache_get (size_t tc_idx) tcache_entry *e = tcache->entries[tc_idx]; assert (tc_idx < TCACHE_MAX_BINS); assert (tcache->entries[tc_idx] > 0); + + /* Check for potential buffer overrun that altered + the chunk size by ensuring it's slot is correct + for it's size. */ + + mchunkptr chunk = mem2chunk (e); + INTERNAL_SIZE_T chunksize = chunksize (chunk); + size_t chunkindex = csize2tidx (chunksize); + + assert (tc_idx == chunkindex); + tcache->entries[tc_idx] = e->next; --(tcache->counts[tc_idx]); return (void *) e; @@ -3627,6 +3638,15 @@ _int_malloc (mstate av, size_t bytes) if ((victim = last (bin)) != bin) { + /* Check for potential buffer overrun that altered + the chunk size by ensuring it's slot is correct + for it's size. */ + + INTERNAL_SIZE_T chunksize = chunksize (victim); + size_t chunkindex = smallbin_index (chunksize); + + assert (idx == chunkindex); + bck = victim->bk; if (__glibc_unlikely (bck->fd != victim)) malloc_printerr ("malloc(): smallbin double linked list corrupted"); @@ -3651,6 +3671,13 @@ _int_malloc (mstate av, size_t bytes) { if (tc_victim != 0) { + /* Check for overrun here too. */ + + INTERNAL_SIZE_T tc_chunksize = chunksize (tc_victim); + size_t tc_chunkindex = smallbin_index (tc_chunksize); + + assert (idx == tc_chunkindex); + bck = tc_victim->bk; set_inuse_bit_at_offset (tc_victim, nb); if (av != &main_arena) diff --git a/malloc/tst-malloc-smallbins-chunksize.c b/malloc/tst-malloc-smallbins-chunksize.c new file mode 100644 index 0000000000..79507641da --- /dev/null +++ b/malloc/tst-malloc-smallbins-chunksize.c @@ -0,0 +1,79 @@ +/* Test and verify that tcache checks for altered chunk size. + 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 + . */ + +/* Bug 22885 reported smallbins wasn't checking for altered chunksize. + After freeing memory the user could alter the size field of the + chunk and smallbins wouldn't check for this. */ + +#include +#include +#include +#include +#include +#include + +/* The following pragma is to prevent the optimize from completely + optimizing out the test code within do_test. */ + +#pragma GCC optimize ("O0") + +static int +do_test (void) +{ + /* Disable fastbins */ + mallopt (M_MXFAST, 0); + + /* The size of 100 was arbitrarily chosen, but needs to be small + enough to use smallbins and large enough to not use a fastbin. */ + char *ptr = (char *) malloc (100); + + /* This prevents coalescing. */ + (void) malloc (100); + + /* This will free a chunk in the middle of the heap so that we + don't merge the allocation back into the top chunk. This + chunk will be placed in the unsorted bin. */ + free (ptr); + + /* This triggers processing of the unsorted bin (around line 3740 + in _int_malloc, look for "for (;; )") which moves the chunk + we just freed to a smallbin. 10000 is an arbitrarily chosen + size that is at least larger than a smallbin size. */ + (void) malloc (10000); + + /* We are corrupting the size field of the chunk in the smallbin, + by increasing it's size by 16 bytes. */ + ptr -= sizeof (size_t); + size_t val = (size_t) *ptr; + size_t *stptr = (size_t *) ptr; + val += 16; + *stptr = val; + + /* This attempts to reuse the chunk we just corrupted. */ + (void) malloc (100); + + /* Execution should not reach here, if FAIL_RET executes look at + malloc/malloc.c _int_malloc, "assert (idx == chunkindex)". */ + FAIL_RET ("no assertion occurred in malloc due to altered chunk size"); + return (0); +} + +#pragma GCC optimize ("O3") + +#define EXPECTED_SIGNAL SIGABRT +#include diff --git a/malloc/tst-malloc-smallbins2tcache-chunksize.c b/malloc/tst-malloc-smallbins2tcache-chunksize.c new file mode 100644 index 0000000000..d29530be4b --- /dev/null +++ b/malloc/tst-malloc-smallbins2tcache-chunksize.c @@ -0,0 +1,112 @@ +/* Test and verify that tcache checks for altered chunk size. + 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 + . */ + +/* Bug 22885 reported smallbins wasn't checking for altered chunksize. + After freeing memory the user could alter the size field of the + chunk and smallbins transfer to tcache wouldn't check for this. */ + +#include +#include +#include +#include +#include +#include + +/* The following pragma is to prevent the optimize from completely + optimizing out the test code within do_test. */ + +#pragma GCC optimize ("O0") + +static int +do_test (void) +{ + /* Disable fastbins */ + mallopt (M_MXFAST, 0); + + /* Allocate a bunch of chunks of size 100, this size was arbitrarily + chosen to be small enough to use smallbin and large enough to not + use a fastbin. */ + /* The allocations below that we do not retain pointers to are used + to prevent coalescing. */ + char *ptr1 = (char *) malloc (100); + (void) malloc (100); + char *ptr2 = (char *) malloc (100); + (void) malloc (100); + char *ptr3 = (char *) malloc (100); + (void) malloc (100); + char *ptr4 = (char *) malloc (100); + (void) malloc (100); + + char *ptr5 = (char *) malloc (100); + (void) malloc (100); + char *ptr6 = (char *) malloc (100); + (void) malloc (100); + char *ptr7 = (char *) malloc (100); + (void) malloc (100); + char *ptr8 = (char *) malloc (100); + (void) malloc (100); + + /* tcache size is set to 4, so these 4 will be moved into tcache. */ + free (ptr5); + free (ptr6); + free (ptr7); + free (ptr8); + + /* These 4 will go into smallbins eventually. */ + free (ptr1); + free (ptr2); + free (ptr3); + free (ptr4); + + /* This triggers processing of the unsorted bin (around line 3740 + in _int_malloc, look for "for (;; )") which moves the chunks + we just freed to a smallbin. 10000 is an arbitrarily chosen + size that is at least larger than a smallbin size. */ + (void) malloc (10000); + + /* We are corrupting the size field of the chunk in the smallbin, + by increasing it's size by 16 bytes. */ + char *ptr = ptr2; + ptr -= sizeof (size_t); + size_t val = (size_t) *ptr; + size_t *stptr = (size_t *) ptr; + val += 16; + *stptr = val; + + /* Malloc all of the 8 chunks again but one of the allocations + will assert before we get all the chunks, because of the corruption + done just above. */ + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + (void) malloc (100); + + /* Execution should not reach here, if FAIL_RET executes look at + malloc/malloc.c _int_malloc, "assert (idx == tc_chunkindex)". */ + FAIL_RET ("no assertion occurred in malloc due to altered chunk size"); + return (0); +} + +#pragma GCC optimize ("O3") + +#define EXPECTED_SIGNAL SIGABRT +#include diff --git a/malloc/tst-malloc-tcache-chunksize.c b/malloc/tst-malloc-tcache-chunksize.c new file mode 100644 index 0000000000..da63e8cde4 --- /dev/null +++ b/malloc/tst-malloc-tcache-chunksize.c @@ -0,0 +1,63 @@ +/* Test and verify that tcache checks for altered chunk size. + 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 + . */ + +/* Bug 22885 reported tcache wasn't checking for altered chunksize. + After freeing memory the user could alter the size field of the + chunk and tcache wouldn't check for this. */ + +#include +#include +#include +#include +#include +#include + +/* The following pragma is to prevent the compiler from completely + optimizing out the test code within do_test. */ + +#pragma GCC optimize ("O0") + +static int +do_test (void) +{ + /* The size of 100 was arbitrarily chosen, but needs to be small + enough to use tcache. */ + char *ptr = (char *) malloc (100); + free (ptr); + + /* We are corrupting the size field of the chunk in the tcache, + by increasing it's size by 8 bytes. */ + ptr -= sizeof (size_t); + size_t val = (size_t) *ptr; + size_t *stptr = (size_t *) ptr; + val += 8; + *stptr = val; + + /* This attempts to reuse the chunk we just corrupted. */ + (void) malloc (100); + + /* Execution should not reach here, if FAIL_RET executes look at + malloc/malloc.c tcache_get, "assert (tc_idx == chunksize)". */ + FAIL_RET ("no assertion occurred in malloc due to altered chunk size"); + return (0); +} + +#pragma GCC optimize ("O3") + +#define EXPECTED_SIGNAL SIGABRT +#include