malloc: Do not corrupt the top of a threaded heap if top chunk is MINSIZE [BZ #18502]

mksquashfs was reported in openSUSE to be causing segmentation faults when
creating installation images. Testing showed that mksquashfs sometimes
failed and could be reproduced within 10 attempts. The core dump looked
like the heap top was corrupted and was pointing to an unmapped area. In
other cases, this has been due to an application corrupting glibc structures
but mksquashfs appears to be fine in this regard.

The problem is that heap_trim is "growing" the top into unmapped space.
If the top chunk == MINSIZE then top_area is -1 and this check does not
behave as expected due to a signed/unsigned comparison

  if (top_area <= pad)
    return 0;

The next calculation extra = ALIGN_DOWN(top_area - pad, pagesz) calculates
extra as a negative number which also is unnoticed due to a signed/unsigned
comparison. We then call shrink_heap(heap, negative_number) which crashes
later. This patch adds a simple check against MINSIZE to make sure extra
does not become negative. It adds a cast to hint to the reader that this
is a signed vs unsigned issue.

Without the patch, mksquash fails within 10 attempts. With it applied, it
completed 1000 times without error. The standard test suite "make check"
showed no changes in the summary of test results.

diff --git a/ChangeLog b/ChangeLog
index b502c78f894b41986ba758b4cee879ac5830e2a8..19cccdce62c10944606607fd0609c59291d4d2c0 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2015-06-26  Mel Gorman  <mgorman@suse.de>
+
+       [BZ #18502]
+       * malloc/arena.c (heap_trim): Don't try to shrink a heap that is
+       already minimal.
+
 2015-06-26  Matthew Fortune  <matthew.fortune@imgtec.com>
 
        * elf/elf.h (DT_MIPS_RLD_MAP_REL): New macro.

diff --git a/NEWS b/NEWS
index 24f8c271387115c98dfeba7569655a5bf65d8069..6bfb82275dd37e563cf9a2fdae619488ecb96723 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -22,10 +22,10 @@ Version 2.22
   18217, 18219, 18220, 18221, 18234, 18244, 18245, 18247, 18287, 18319,
   18324, 18333, 18346, 18371, 18397, 18409, 18410, 18412, 18418, 18422,
   18434, 18444, 18468, 18469, 18470, 18479, 18483, 18495, 18496, 18497,
-  18498, 18507, 18512, 18513, 18519, 18520, 18522, 18527, 18528, 18529,
-  18530, 18532, 18533, 18534, 18536, 18539, 18540, 18542, 18544, 18545,
-  18546, 18547, 18549, 18553, 18558, 18569, 18583, 18585, 18586, 18593,
-  18594, 18602.
+  18498, 18502, 18507, 18512, 18513, 18519, 18520, 18522, 18527, 18528,
+  18529, 18530, 18532, 18533, 18534, 18536, 18539, 18540, 18542, 18544,
+  18545, 18546, 18547, 18549, 18553, 18558, 18569, 18583, 18585, 18586,
+  18593, 18594, 18602.
 
 * Cache information can be queried via sysconf() function on s390 e.g. with
   _SC_LEVEL1_ICACHE_SIZE as argument.

diff --git a/malloc/arena.c b/malloc/arena.c
index 2466697d1aa7764d3be9cfee35c1330d1a443bf5..21ecc5a13791e1156996448f53a3074f8805c87a 100644 (file)
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -699,7 +699,7 @@ heap_trim (heap_info *heap, size_t pad)
      by preserving the top pad and at least a page.  */
   top_size = chunksize (top_chunk);
   top_area = top_size - MINSIZE - 1;
-  if (top_area <= pad)
+  if (top_area < 0 || (size_t) top_area <= pad)
     return 0;
 
   extra = ALIGN_DOWN(top_area - pad, pagesz);