[Devel] Re: [BUG] Double free for embedded bitmaps?

[Gwenole Beauchesne]

Hi,

Users are experiencing some crashes with OpenOffice.org dynamically linked
against system freetype 2.1.3. This occurs only if OOo requests to handle
embedded bitmaps.

This is reported in MDK Bugzilla #2785:
<http://qa.mandrakesoft.com/show_bug.cgi?id=2785>

Similar, I believe, to Debian #183272:
<http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=183272>

Sorry for the terminology used, I am really not familiar with freetype2.

## Random facts:

1) I have verified that it works with freetype2 2.1.3rc2 but started to 
break with 2.1.3rc3. Someone reported that it would work by reverting:
<http://www.freetype.org/cgi-bin/viewcvs.cgi/freetype2/src/base/ftobjs.c.diff?r1=1.153&r2=1.154>

2) If I understand things correctly, it is possible to inherit a 
bitmap.buffer from an FT_GlyphSlot to an FT_BitmapGlyph, right?
<freetype2>/src/base/ftglyph.c (ft_bitmap_glyph_init)

3) However, in src/sfnt/ttsbit.c (tt_face_load_sbit_image) we are first 
free'ing map->buffer if and only if the slot owns the buffer. But later, 
if Load_SBit_Image() succeeds, we unconditionnally says that the slot 
still owns the buffer.

Couldn't this scenario exist?
- we have a new FT_BitmapGlyph inheriting buffer from an FT_GlyphSlot
- we then do an FT_Done_Glyph and henceforth freeing the buffer
==> at this stage, we still have the original FT_GlyphSlot bitmap->buffer 
pointing to the area that was just free'd.

4) Another bug was possibly that in the event Load_SBit_Image() we are 
exitting but leaving the possibly just deallocated buffer in a state 
telling the slot owns it.

## Suggested fix:

--- freetype-2.1.3/src/sfnt/ttsbit.c.fix-double-free    2002-09-28 
18:40:57.000000000 +0200
+++ freetype-2.1.3/src/sfnt/ttsbit.c    2003-03-11 21:28:07.000000000 +0100
@@ -1434,7 +1434,17 @@
 
     /* clear the bitmap & load the bitmap */
     if ( face->root.glyph->flags & FT_GLYPH_OWN_BITMAP )
-      FT_FREE( map->buffer );
+    {
+        FT_FREE( map->buffer );
+        face->root.glyph->flags &= ~FT_GLYPH_OWN_BITMAP;
+    }
+    else
+    {
+        /* assume someone else stole us the buffer and handles its
+           deallocation. In that case, let Load_SBit_Image() allocate
+           a new buffer that this slot will own.  */
+        map->buffer = NULL;
+    }
 
     map->rows = map->pitch = map->width = 0;
 
However, the comment said /* clear the bitmap & load the bitmap */. i.e. 
does it assume that if the slot doesn't own the buffer, map->buffer was 
expected to be NULL, so that it is allocated in Load_SBit_Image()?

Any ideas?

Thanks,
Gwenole.

PS: Sorry but I am not subscribed to the mailing-list.