From 6eb8c1d071c7051486183e1e3e8528c8ec240197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Stefan=20P=C3=B6schel?= Date: Fri, 5 May 2017 11:31:39 +0200 Subject: [PATCH] Fix Memleak in AcquireVirtualMemory The memleak is introduced by the Debian patch "0095-Fix-multiple-out-of-bound-problem.patch" that is applied on top of the original ImageMagick source: https://sources.debian.net/patches/imagemagick/8:6.8.9.9-5%2Bdeb8u8/0095-Fix-multiple-out-of-bound-problem.patch/ The patch - according to its name and its commit message - fixes some OOB problems, but unfortunately also modifies other behaviour, in constrast to the original commit that it refers to: https://github.com/ImageMagick/ImageMagick/commit/2174484dfa68a594e2f9ad17f46217b6120db18d The memleak happens in the function "AcquireVirtualMemory" in "/magick/memory.c": https://sources.debian.net/src/imagemagick/8:6.8.9.9-5%2Bdeb8u8/magick/memory.c/#L589 In that function, the different usable memory sources are queried one-after-another until memory from one source can be acquired. This means that the function does NOT try to acquire memory from source X, if the acquisition from source X-1 already succeeded. Unfortunately the mentioned patch contradicts this behaviour for the first memory source that uses the "AcquireAlignedMemory" function. Hence memory from the second source is acquired as well (and later freed) while overwriting the pointer memory_info->blob that points to the previously acquired memory. Thus the memory from the first source is never freed. I attach a patch that fixes the memleak. Valgrind then displays: ==5428== LEAK SUMMARY: ==5428== definitely lost: 0 bytes in 0 blocks ==5428== indirectly lost: 0 bytes in 0 blocks ==5428== possibly lost: 1,352 bytes in 18 blocks ==5428== still reachable: 120,708 bytes in 228 blocks But possibly it rather makes sense to clean up that Debian patch in general, and to separate the OOB fixes and the other changes (that needs to be corrected). bug-debian: https://bugs.debian.org/859772 bug-ubuntu: https://bugs.launchpad.net/ubuntu/+source/imagemagick/+bug/1680543 --- magick/memory.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/magick/memory.c b/magick/memory.c index 698a14073..33051719f 100644 --- a/magick/memory.c +++ b/magick/memory.c @@ -587,7 +587,8 @@ MagickExport MemoryInfo *AcquireVirtualMemory(const size_t count, RelinquishMagickResource(MemoryResource,extent); } RelinquishMagickResource(MemoryResource,extent); - if (AcquireMagickResource(MapResource,extent) != MagickFalse) + if ((memory_info->blob == NULL) && + (AcquireMagickResource(MapResource,extent) != MagickFalse)) { /* Heap memory failed, try anonymous memory mapping.