[Bug 1680543] Re: Memleak in AcquireVirtualMemory

[Bug Watch Updater]

** Changed in: imagemagick (Debian)
       Status: Unknown => Fix Released

-- 
You received this bug notification because you are a member of Ubuntu
Foundations Bugs, which is subscribed to imagemagick in Ubuntu.
https://bugs.launchpad.net/bugs/1680543

Title:
  Memleak in AcquireVirtualMemory

Status in imagemagick package in Ubuntu:
  Fix Released
Status in imagemagick package in Debian:
  Fix Released

Bug description:
  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.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/imagemagick/+bug/1680543/+subscriptions