| Message ID | 20150608123613.GO26425@suse.de |
|---|---|
| State | New |
| Headers | show |
On Mon, Jun 08, 2015 at 01:36:13PM +0100, Mel Gorman wrote: > 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. > > 2015-06-08 Mel Gorman <mgorman@suse.de> > > [BZ #18502] > * malloc/arena.c: Avoid corruption of the top of heaps for threads Ping as it's been one week since the last submission.
On Mon, Jun 15, 2015 at 09:02:24AM +0100, Mel Gorman wrote: > On Mon, Jun 08, 2015 at 01:36:13PM +0100, Mel Gorman wrote: > > 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. > > > > 2015-06-08 Mel Gorman <mgorman@suse.de> > > > > [BZ #18502] > > * malloc/arena.c: Avoid corruption of the top of heaps for threads > > Ping as it's been one week since the last submission. > Another ping as it's been one week since the last ping.
Please install. Andreas.
diff --git a/malloc/arena.c b/malloc/arena.c index 2466697d1aa7..729a09e1d846 100644 --- 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);
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. 2015-06-08 Mel Gorman <mgorman@suse.de> [BZ #18502] * malloc/arena.c: Avoid corruption of the top of heaps for threads --- malloc/arena.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)