bugThe FreeType Project - Bugs: bug #30648, Crash - SIGSEGV -...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #30648: Crash - SIGSEGV - user-controllable read in base/ftobjs.c:425 - T42 font

Submitter:  Robert Swiecki <jagger>
Submitted:  Wed 04 Aug 2010 02:31:53 PM UTC
   
 
Severity:  3 - Normal Item Group:  Crash
Status:  Fixed Privacy:  Public
Assigned to:  mpsuzuki Open/Closed:  Closed
Planned Release:  2.4.2

Jump to the original submission

Fri 06 Aug 2010 06:18:46 PM UTC, comment #6: 

Works fine for me!  Thanks for the quick fix.

Werner LEMBERG <wl>
Group administrator
Fri 06 Aug 2010 04:52:02 PM UTC, comment #5: 

Just I've committed following patch to GIT head (idea B style).
Please check the latest source on GIT.

 diff --git a/ChangeLog b/ChangeLog
 index 898ac3f..3869fc2 100644
 --- a/ChangeLog
 +++ b/ChangeLog
 @@ -1,3 +1,13 @@
 +2010-08-06  suzuki toshiya  <mpsuzuki@hiroshima-u.ac.jp>
 +
 + Fix Savannah bug #30648.
 +
 + * src/base/ftobjs.c (FT_Done_Library): Specify the order of font
 + drivers in face closing process. Type42 faces should be closed
 + before TrueType faces, because a Type42 face refers another
 + internal TrueType face which is created from sfnt[] array on the
 + memory.
 +
  2010-08-06  Yuriy Kaminskiy  <yumkam@mail.ru>
 
  [raster] Fix valgrind warning.
 diff --git a/src/base/ftobjs.c b/src/base/ftobjs.c
 index 13c126f..7582f5b 100644
 --- a/src/base/ftobjs.c
 +++ b/src/base/ftobjs.c
 @@ -4416,26 +4416,42 @@
       *  - 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
 +     *
 +     * Some faces are dependent with other faces, like Type42 faces that
 +     * depends on TrueType face synthesized internally.
 +     * The order of driver should be described in driver_name[].
       */
      {
 -      FT_UInt  n;
 +      FT_UInt  m, n;
 +      const char* driver_name[] = { "type42", NULL };
 
 
 -      for ( n = 0; n < library->num_modules; n++ )
 +      for ( m = 0;
 +            m < sizeof( driver_name ) / sizeof( driver_name[0] );
 +            m ++ )
        {
 -        FT_Module  module = library->modules[n];
 -        FT_List    faces;
 +        for ( n = 0; n < library->num_modules; n++ )
 +        {
 +          FT_Module    module = library->modules[n];
 +          const char*  module_name = module->clazz->module_name;
 +          FT_List      faces;
 
 
 -        if ( ( module->clazz->module_flags & FT_MODULE_FONT_DRIVER ) == 0 )
 -          continue;
 +          if ( driver_name[m]                                &&
 +               0 != ft_strcmp( module_name, driver_name[m] ) )
 +            continue;
 
 -        faces = &FT_DRIVER(module)->faces_list;
 -        while ( faces->head )
 -        {
 -          FT_Done_Face( FT_FACE( faces->head->data ) );
 -          if ( faces->head )
 -            FT_TRACE0(( "FT_Done_Library: failed to free some faces\n" ));
 +          if ( ( module->clazz->module_flags & FT_MODULE_FONT_DRIVER ) == 0 )
 +            continue;
 +
 +          FT_TRACE7(( "FT_Done_Library: close faces for %s\n", module_name ));
 +          faces = &FT_DRIVER(module)->faces_list;
 +          while ( faces->head )
 +          {
 +            FT_Done_Face( FT_FACE( faces->head->data ) );
 +            if ( faces->head )
 +              FT_TRACE0(( "FT_Done_Library: failed to free some faces\n" ));
 +          }
          }
        }
      }

suzuki toshiya <mpsuzuki>
Group administrator
Fri 06 Aug 2010 03:40:16 PM UTC, comment #4: 

Ah, the double free issue for Type42 is already described
in the comment around how to remove driver modules, like
this:

   4443     /* Close all other modules in the library */
   4444 #if 1
   4445     /* XXX Modules are removed in the reversed order so that  */
   4446     /* type42 module is removed before truetype module.  This */
   4447     /* avoids double free in some occasions.  It is a hack.   */
   4448     while ( library->num_modules > 0 )
   4449       FT_Remove_Module( library,
   4450                         library->modules[library->num_modules - 1] );
   4451 #else
   4452     {
   4453       FT_UInt  n;
   4454
   4455
   4456       for ( n = 0; n < library->num_modules; n++ )
   4457       {
   4458         FT_Module  module = library->modules[n];
   4459
   4460
   4461         if ( module )
   4462         {
   4463           Destroy_Module( module );
   4464           library->modules[n] = 0;
   4465         }
   4466       }
   4467     }
   4468 #endif

It seems that my eyepits are just holes.

suzuki toshiya <mpsuzuki>
Group administrator
Fri 06 Aug 2010 03:27:44 PM UTC, comment #3: 


>B) freeing the faces in reversal order (younger is freed for earlier,
>older is freed in later)


This idea is incorrect. The face objects are listed in per-driver
linked list, so reversal order in per-driver linked list does not
solve this issue. The idea should be freeing faces in Type42 driver
in earlier and freeing faces in TrueType driver. Also I have to
care that the order of the driver in library->modules[] is not
same always (although TrueType driver is earlier than Type42,
when configured by default)

suzuki toshiya <mpsuzuki>
Group administrator
Fri 06 Aug 2010 02:44:31 PM UTC, comment #2: 

I'm afraid that normal Type42 font can crash ftbench.
When I convert LiberationMono-Regular.ttf (by RedHat)
to Type42 by fontforge, and give it to ftbench, it is crashed.

I guess a double free occurs. The rough sketch of
my understanding is following:

1) FT_Open_Face() tries to open a font face from Type42 file
2) FT_Open_Face() makes a memory-based stream from the sfnt[]
array in Type42, and invoke another FT_Open_Face() internally.
3) The 2nd FT_Open_Face() opens a face from TrueType memory image.
4) The 1st FT_Open_Face() creates another face with extra info out
of the sfnt[] array in Type42 stream.
5) The 1st and 2nd faces create their own FT_GlyphSlots.
6) The glyph slot in the 2nd face is allocated for the 2nd face,
but its "ttslot" refers the 1st face's glyphslot.
7) FT_Done_Library invokes FT_Done_Face(). For first, the 1st face
is processed, then the 2nd face is processed.
8) When the 2nd face is being freed, its "ttslot" is tried to be freed.
But it is already freeed when the 1st face was freed. The doube free
occurs.

I have 2 ideas to fix this issue.
A) referring from the 2nd face glyph slot to the 1st face glyph slot
is not good idea, it should be fixed, or, reference counter to prevent
freeing the slot when it is referred by other faces.
B) freeing the faces in reversal order (younger is freed for earlier,
older is freed in later)

suzuki toshiya <mpsuzuki>
Group administrator
Thu 05 Aug 2010 08:19:58 AM UTC, comment #1: 

Thank you for report, I will take a look.

suzuki toshiya <mpsuzuki>
Group administrator
Wed 04 Aug 2010 02:31:53 PM UTC, original submission:  

Font file: http://alt.swiecki.net/j/f/sigsegv28.ttf

$ gdb ./ftbench
(gdb) r -c 1 sigsegv28.ttf
Starting program: /usr/local/google/fuzz/ttf/ftbench -c 1 sigsegv28.ttf
Load                      : 18.933 us/op
Load_Advances (Normal)    : 0.037 us/op
Load_Advances (Fast)      : 0.324 us/op
Render                    : 11.847 us/op
Get_Glyph                 : 1.076 us/op
Get_CBox                  : 0.464 us/op
Get_Char_Index            : 0.126 us/op
Iterate CMap              : 113.964 us/op
New_Face                  : 17069.817 us/op
Embolden                  : 0.052 us/op

Program received signal SIGSEGV, Segmentation fault.
0x0000000000405385 in FT_Done_GlyphSlot (slot=0x6ae230) at /usr/local/google/src/freetype2/src/base/ftobjs.c:425
425       FT_Memory     memory = driver->root.memory;

(gdb) p driver
$1 = (FT_Driver) 0x68686561  <-- This is some ASCII text 'hhea'

$ valgrind ./ftbench -c 1 sigsegv28.ttf
==31804== Memcheck, a memory error detector
==31804== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==31804== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==31804== Command: ./ftbench -c 1 sigsegv28.ttf
==31804==
Load                      : 748.999 us/op
Load_Advances (Normal)    : 4.196 us/op
Load_Advances (Fast)      : 190.327 us/op
Render                    : 381.692 us/op
Get_Glyph                 : 77.722 us/op
Get_CBox                  : 22.184 us/op
Get_Char_Index            : 2.549 us/op
Iterate CMap              : 2435.923 us/op
New_Face                  : 224760.056 us/op
Embolden                  : 2.846 us/op
==31804== Invalid read of size 8
==31804==    at 0x405372: FT_Done_GlyphSlot (ftobjs.c:424)
==31804==    by 0x440586: T42_GlyphSlot_Done (t42objs.c:578)
==31804==    by 0x4051B6: ft_glyphslot_done (ftobjs.c:348)
==31804==    by 0x4053EF: FT_Done_GlyphSlot (ftobjs.c:443)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5422428 is 8 bytes inside a block of size 304 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x405402: FT_Done_GlyphSlot (ftobjs.c:444)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x405376: FT_Done_GlyphSlot (ftobjs.c:424)
==31804==    by 0x440586: T42_GlyphSlot_Done (t42objs.c:578)
==31804==    by 0x4051B6: ft_glyphslot_done (ftobjs.c:348)
==31804==    by 0x4053EF: FT_Done_GlyphSlot (ftobjs.c:443)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5420c50 is 176 bytes inside a block of size 1,384 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x406005: destroy_face (ftobjs.c:906)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x405399: FT_Done_GlyphSlot (ftobjs.c:432)
==31804==    by 0x440586: T42_GlyphSlot_Done (t42objs.c:578)
==31804==    by 0x4051B6: ft_glyphslot_done (ftobjs.c:348)
==31804==    by 0x4053EF: FT_Done_GlyphSlot (ftobjs.c:443)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5422428 is 8 bytes inside a block of size 304 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x405402: FT_Done_GlyphSlot (ftobjs.c:444)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x40539D: FT_Done_GlyphSlot (ftobjs.c:432)
==31804==    by 0x440586: T42_GlyphSlot_Done (t42objs.c:578)
==31804==    by 0x4051B6: ft_glyphslot_done (ftobjs.c:348)
==31804==    by 0x4053EF: FT_Done_GlyphSlot (ftobjs.c:443)
==31804==    by 0x405EEB: destroy_face (ftobjs.c:874)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5420c38 is 152 bytes inside a block of size 1,384 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x406005: destroy_face (ftobjs.c:906)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x4103C1: FT_List_Find (ftutil.c:247)
==31804==    by 0x4404B7: T42_Size_Done (t42objs.c:542)
==31804==    by 0x405D9F: destroy_size (ftobjs.c:822)
==31804==    by 0x410667: FT_List_Finalize (ftutil.c:402)
==31804==    by 0x405F21: destroy_face (ftobjs.c:877)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5420c68 is 200 bytes inside a block of size 1,384 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x406005: destroy_face (ftobjs.c:906)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x407F6D: FT_Done_Face (ftobjs.c:2286)
==31804==    by 0x440016: T42_Face_Done (t42objs.c:407)
==31804==    by 0x405F7F: destroy_face (ftobjs.c:892)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5420c50 is 176 bytes inside a block of size 1,384 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x406005: destroy_face (ftobjs.c:906)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804== Invalid read of size 8
==31804==    at 0x407F81: FT_Done_Face (ftobjs.c:2288)
==31804==    by 0x440016: T42_Face_Done (t42objs.c:407)
==31804==    by 0x405F7F: destroy_face (ftobjs.c:892)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==  Address 0x5420c50 is 176 bytes inside a block of size 1,384 free'd
==31804==    at 0x4C280BD: free (vg_replace_malloc.c:366)
==31804==    by 0x47EDBE: ft_free (ftsystem.c:158)
==31804==    by 0x410296: ft_mem_free (ftutil.c:171)
==31804==    by 0x406005: destroy_face (ftobjs.c:906)
==31804==    by 0x408002: FT_Done_Face (ftobjs.c:2300)
==31804==    by 0x40B1EF: FT_Done_Library (ftobjs.c:4394)
==31804==    by 0x402FBC: FT_Done_FreeType (ftinit.c:243)
==31804==    by 0x402A32: main (ftbench.c:891)
==31804==
==31804==
==31804== HEAP SUMMARY:
==31804==     in use at exit: 0 bytes in 0 blocks
==31804==   total heap usage: 6,334 allocs, 6,334 frees, 1,756,465 bytes allocated
==31804==
==31804== All heap blocks were freed -- no leaks are possible
==31804==
==31804== For counts of detected and suppressed errors, rerun with: -v
==31804== ERROR SUMMARY: 7 errors from 7 contexts (suppressed: 4 from 4)

Robert Swiecki <jagger>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #21158:  LiberationMono.t42 added by mpsuzuki (117KiB - application/octet-stream)
file #21138:  sigsegv28.ttf added by jagger (259KiB - application/x-font-ttf)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by mpsuzuki (Posted a comment)
  • -email is unavailable- added by jagger (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2010-08-06 wl StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2010-08-06 mpsuzuki StatusConfirmed Ready For Test
        Planned ReleaseNone 2.4.2
    2010-08-06 mpsuzuki Attached File- Added LiberationMono.t42, #21158
    2010-08-05 mpsuzuki StatusNone Confirmed
        Assigned toNone mpsuzuki
    2010-08-04 jagger Attached File- Added sigsegv28.ttf, #21138

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code