This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCHv2/22885] Detect altered malloc chunks
- From: Mike Thomas <michael at thomii dot com>
- To: <libc-alpha at sourceware dot org>
- Date: Mon, 12 Nov 2018 15:38:09 -0500
- Subject: [PATCHv2/22885] Detect altered malloc chunks
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
+ <http://www.gnu.org/licenses/>. */
+
+/* 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 <stdio.h>
+#include <stdlib.h>
+#include <malloc.h>
+#include <string.h>
+#include <signal.h>
+#include <support/check.h>
+
+/* 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 <support/test-driver.c>
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
+ <http://www.gnu.org/licenses/>. */
+
+/* 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 <stdio.h>
+#include <stdlib.h>
+#include <malloc.h>
+#include <string.h>
+#include <signal.h>
+#include <support/check.h>
+
+/* 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 <support/test-driver.c>
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
+ <http://www.gnu.org/licenses/>. */
+
+/* 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 <stdio.h>
+#include <stdlib.h>
+#include <malloc.h>
+#include <string.h>
+#include <signal.h>
+#include <support/check.h>
+
+/* 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 <support/test-driver.c>