Discussion:
malloc: Add another test for tcache double free check.
(too old to reply)
DJ Delorie
2018-12-01 03:32:00 UTC
Permalink
This one tests for RHBZ#1652495 where the double free
test didn't check the tcache bin bounds before dereferencing
the bin.

* malloc/tst-tcfree3.c: New.
* malloc/Makefile: Add it.

diff --git a/malloc/Makefile b/malloc/Makefile
index e6dfbfc14c..388cf7e9ee 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -38,7 +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 \
+ tst-tcfree1 tst-tcfree2 tst-tcfree3 \

tests-static := \
tst-interpose-static-nothread \
diff --git a/malloc/tst-tcfree3.c b/malloc/tst-tcfree3.c
new file mode 100644
index 0000000000..1db6d2b220
--- /dev/null
+++ b/malloc/tst-tcfree3.c
@@ -0,0 +1,56 @@
+/* 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
+ <http://www.gnu.org/licenses/>. */
+
+#include <malloc.h>
+#include <string.h>
+
+/* Prevent GCC from optimizing away any malloc/free pairs. */
+#pragma GCC optimize ("O0")
+
+static int
+do_test (void)
+{
+ /* Do two allocation of any size that fit in tcache, and one that
+ doesn't. */
+ int ** volatile a = malloc (32);
+ int ** volatile b = malloc (32);
+ /* This is just under the MMAP threshold. */
+ int ** volatile c = malloc (127 * 1024);
+
+ /* The invalid "tcache bucket" we might dereference will likely end
+ up somewhere within this memory block, so make all the accidental
+ "next" pointers cause segfaults. RHBZ#1652495. */
+ memset (c, 0xff, 127 * 1024);
+
+ free (a); // puts in tcache
+
+ /* A is now free and contains the key we use to detect in-tcache.
+ Copy the key to the other chunks. */
+ memcpy (b, a, 32);
+ memcpy (c, a, 32);
+
+ /* This free tests the "are we in the tcache already" loop with a
+ VALID bin but "coincidental" matching key. */
+ free (b); // should NOT abort
+ /* This free tests the "is it a valid tcache bin" test. */
+ free (c); // should NOT abort
+
+ return 0;
+}
+
+#include <support/test-driver.c>
Carlos O'Donell
2018-12-01 03:58:46 UTC
Permalink
Post by DJ Delorie
This one tests for RHBZ#1652495 where the double free
test didn't check the tcache bin bounds before dereferencing
the bin.
OK for master if you tested it catches the bug, and you fixup
the comments below.

Reviewed-by: Carlos O'Donell <***@redhat.co>

Did you test that the test crashes without Florian's fix, but
with your doubl-free code enabled?

Needs a bug number in ChangeLog:
[BZ #23907]
Post by DJ Delorie
* malloc/tst-tcfree3.c: New.
* malloc/Makefile: Add it.
diff --git a/malloc/Makefile b/malloc/Makefile
index e6dfbfc14c..388cf7e9ee 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -38,7 +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 \
+ tst-tcfree1 tst-tcfree2 tst-tcfree3 \
Ok.
Post by DJ Delorie
tests-static := \
tst-interpose-static-nothread \
diff --git a/malloc/tst-tcfree3.c b/malloc/tst-tcfree3.c
new file mode 100644
index 0000000000..1db6d2b220
--- /dev/null
+++ b/malloc/tst-tcfree3.c
@@ -0,0 +1,56 @@
+/* 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
+ <http://www.gnu.org/licenses/>. */
+
+#include <malloc.h>
+#include <string.h>
+
+/* Prevent GCC from optimizing away any malloc/free pairs. */
+#pragma GCC optimize ("O0")
+
+static int
+do_test (void)
+{
+ /* Do two allocation of any size that fit in tcache, and one that
+ doesn't. */
+ int ** volatile a = malloc (32);
+ int ** volatile b = malloc (32);
+ /* This is just under the MMAP threshold. */
s/MMAP/mmap/g
Post by DJ Delorie
+ int ** volatile c = malloc (127 * 1024);
+
+ /* The invalid "tcache bucket" we might dereference will likely end
+ up somewhere within this memory block, so make all the accidental
+ "next" pointers cause segfaults. RHBZ#1652495. */
s/RHBZ#1652495/BZ #23907/g
Post by DJ Delorie
+ memset (c, 0xff, 127 * 1024);
+
+ free (a); // puts in tcache
+
+ /* A is now free and contains the key we use to detect in-tcache.
+ Copy the key to the other chunks. */
+ memcpy (b, a, 32);
+ memcpy (c, a, 32);
+
+ /* This free tests the "are we in the tcache already" loop with a
+ VALID bin but "coincidental" matching key. */
+ free (b); // should NOT abort
+ /* This free tests the "is it a valid tcache bin" test. */
+ free (c); // should NOT abort
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
Cheers,
Carlos.
DJ Delorie
2018-12-01 04:14:08 UTC
Permalink
Post by Carlos O'Donell
Did you test that the test crashes without Florian's fix, but
with your doubl-free code enabled?
Yes. That took a while, and the result was the memset in the test. I
spent a while trying to get the "invalid bin" to be outside mapped
space, to no avail.
Post by Carlos O'Donell
[BZ #23907]
Done.
Post by Carlos O'Donell
Post by DJ Delorie
+ /* This is just under the MMAP threshold. */
s/MMAP/mmap/g
Fixed.
Post by Carlos O'Donell
s/RHBZ#1652495/BZ #23907/g
Fixed.
DJ Delorie
2018-12-08 00:35:28 UTC
Permalink
Post by Carlos O'Donell
OK for master if you tested it catches the bug, and you fixup
the comments below.
Thanks! Pushed.

Loading...