Memleak in AcquireVirtualMemory

[Stefan Pöschel]

The latest ImageMagick package (version 8:6.8.9.9-7ubuntu5.6) in Ubuntu 16.04 is affected by a memleak that occurs when images of certain formats are loaded or written. As it affects JPEG and PNG, it will presumably affect most people using ist.

To reproduce it, just take e.g. an image test.jpg and call "convert test.jpg test.png". Valgrind here shows:

[...]
==8396== 1,408 bytes in 1 blocks are definitely lost in loss record 237 of 248
==8396== at 0x4C2FFC6: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8396== by 0x4C300D1: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8396== by 0x4FA11DA: AcquireAlignedMemory (memory.c:259)
==8396== by 0x4FA14EC: AcquireVirtualMemory (memory.c:583)
==8396== by 0x511FF5A: ReadJPEGImage (jpeg.c:1271)
==8396== by 0x4EF86D7: ReadImage (constitute.c:547)
==8396== by 0x4EF976A: ReadImages (constitute.c:853)
==8396== by 0x5541E17: ConvertImageCommand (convert.c:622)
==8396== by 0x55AE466: MagickCommandGenesis (mogrify.c:168)
==8396== by 0x4008B6: ConvertMain (convert.c:81)
==8396== by 0x4008B6: main (convert.c:92)
==8396==
==8396== 1,408 bytes in 1 blocks are definitely lost in loss record 238 of 248
==8396== at 0x4C2FFC6: memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8396== by 0x4C300D1: posix_memalign (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8396== by 0x4FA11DA: AcquireAlignedMemory (memory.c:259)
==8396== by 0x4FA14EC: AcquireVirtualMemory (memory.c:583)
==8396== by 0x512C787: WriteOnePNGImage (png.c:10750)
==8396== by 0x5130AB1: WritePNGImage (png.c:12052)
==8396== by 0x4EF9BCE: WriteImage (constitute.c:1184)
==8396== by 0x4EFA571: WriteImages (constitute.c:1335)
==8396== by 0x55416DD: ConvertImageCommand (convert.c:3215)
==8396== by 0x55AE466: MagickCommandGenesis (mogrify.c:168)
==8396== by 0x4008B6: ConvertMain (convert.c:81)
==8396== by 0x4008B6: main (convert.c:92)
==8396==
==8396== LEAK SUMMARY:
==8396== definitely lost: 2,816 bytes in 2 blocks
==8396== indirectly lost: 0 bytes in 0 blocks
==8396== possibly lost: 1,352 bytes in 18 blocks
==8396== still reachable: 120,708 bytes in 228 blocks
[...]

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). The reason is that the patch got a bit complex and the patch name does not reflect what the patch actually does.

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "Memleak patch" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

[Stefan Pöschel (stefan-poeschel)]

I noticed that this has been fixed with version 8:6.8.9.9-7ubuntu5.7.