Thanks.  I was aware of the "might be clobbered" warnings but did
not know how to fix them.  I'll apply your suggestion right away.

Don't look for the change on SourceForge any time soon, though.
Get it from Svn or from www.imagemagick.org.  SourceForge has changed
to a social-network-like GUI file release system and it's going to
take me a while to deal with it.  Especially with their light cyan
text on lighter-cyan background, I can't even see very well what is
going on.

Glenn

----- Original Message -----
From: "Alexander E. Patrakov" <patra...@gmail.com>
To: magick-developers@imagemagick.org
Sent: Friday, July 24, 2009 2:14:02 AM GMT -05:00 US/Canada Eastern
Subject: [magick-developers] Incorrect use of setjmp in coders/png.c

Hi,

The ReadOnePNGImage() function contains this code:

  QuantumInfo
    *quantum_info;
...
  quantum_info = NULL;
  if (setjmp(ping->jmpbuf))
    {
      /*
        PNG image is corrupt.
      */
...
      if (quantum_info != (QuantumInfo *) NULL)
        quantum_info = DestroyQuantumInfo(quantum_info);
...
      return(GetFirstImageInList(image));
    }
...
  /*
    Convert PNG pixels to pixel packets.
  */
  quantum_info=AcquireQuantumInfo(image_info,image);
  /* use it, possibly call longjmp() ... */
  quantum_info=DestroyQuantumInfo(quantum_info);

And here is a problem, because the longjmp() manual page says:

       The values of automatic variables  are  unspecified  after  a  call  to
       longjmp() if they meet all the following criteria:

       o  they are local to the function that made the corresponding setjmp(3)
          call;

       o  their  values  are  changed  between  the  calls  to  setjmp(3)  and
          longjmp(); and

       o  they are not declared as volatile.

For quantum_info, all three conditions are true. And indeed, some gcc
versions decide to cache it in a register at the time when setjmp() is
called. So, when longjmp() restores that context, it also restores the
register where quantum_info is held (to NULL). Thus,
DestroyQuantumInfo is not called, and there is a memory leak
reproducible on linux, e.g., by passing an incompletely downloaded png
file to BlobToImage(). The same consideration applies to two more
variables.

So, for a quick fix, I propose to change three declarations, as follows:

  Image
    * volatile image;

  QuantumInfo
    * volatile quantum_info;

  unsigned char
    * volatile png_pixels;



-- 
Alexander E. Patrakov
_______________________________________________
Magick-developers mailing list
Magick-developers@imagemagick.org
http://studio.imagemagick.org/mailman/listinfo/magick-developers
_______________________________________________
Magick-developers mailing list
Magick-developers@imagemagick.org
http://studio.imagemagick.org/mailman/listinfo/magick-developers

Reply via email to