[ft-devel] Broken FT_Face data can cause the infinite loop in FT_Done_Library()

[mpsuzuki]

Hi all,

During the development of sfnt-wrapped CID font
support, I found that a broken FT_Face data can
cause an infinite loop in FT_Done_Library().

ftobjs.c
--------
  /* documentation is in ftmodapi.h */

  FT_EXPORT_DEF( FT_Error )
  FT_Done_Library( FT_Library  library )
  {
...

    /* Close all faces in the library.  If we don't do
     * this, we can have some subtle memory leaks.
     * Example:
     *
     *  - the cff font driver uses the pshinter module in cff_size_done
     *  - if the pshinter module is destroyed before the cff font driver,
     *    opened FT_Face objects managed by the driver are not properly
     *    destroyed, resulting in a memory leak
     */
    {
...
      for ( n = 0; n < library->num_modules; n++ )
      {
...
        faces = &FT_DRIVER(module)->faces_list;
        while ( faces->head )
          FT_Done_Face( FT_FACE( faces->head->data ) );
      }
    }

If I pass a broken FT_Face data to FT_Done_Face()
which FT_Done_Face() cannot free,  

        while ( faces->head )
          FT_Done_Face( FT_FACE( faces->head->data ) );

falls into an infinite loop. In my case, some wrong
allocation of the buffer to store the image of PS font
included in sfnt-wrapped CID-keyed font caused this
problem. Nothing to say, FreeType2 cannot handle a broken
FT_Face data, so it is NOT a bug.

But I wonder if there's any case that the first trial
cannot free the font data but the second (or later)
trial can free it. If there's no such possibility,
there might be no requirement to repeat FT_Done_Face()
for the FT_Face data when FT_Done_Face() failed once.
Issue an error immediately, or leaving it without free
(and let the debugger or some detectors of memory leak
handle it) would be slightly easier for debugging
purpose.

If there's no requirement to repeat FT_Done_Face()
for the FT_Face data that once failed, I propose to
replace

        while ( faces->head )
          FT_Done_Face( FT_FACE( faces->head->data ) );

by

        if ( faces->head )
          FT_Done_Face( FT_FACE( faces->head->data ) );

Please give me comments.

Regards,
mpsuzuki