My tests involved exercising both testing and production code that was using FreeType as part of laying out and rasterizing text from TrueType fonts. The output appeared unchanged after the fix, and I didn't see crashing or other behavior suggestive of data corruption. Memory consumption was (unsurprisingly) observed to be lower. I ran some of the testing under valgrind, which did not notice any issues. A quick grep shows these APIs were probably exercised:
FTC_CMapCache_Lookup FTC_CMapCache_New FTC_ImageCache_Lookup FTC_ImageCache_New FTC_Manager_Done FTC_Manager_LookupFace FTC_Manager_New FT_Done_Face FT_Done_FreeType FT_Get_Charmap_Index FT_Get_Kerning FT_Glyph_Get_CBox FT_Glyph_StrokeBorder FT_Glyph_To_Bitmap FT_Glyph_Transform FT_Init_FreeType FT_Load_Glyph FT_Load_Sfnt_Table FT_New_Face FT_New_Memory_Face FT_Open_Face FT_Outline_Copy FT_Outline_Done FT_Outline_Get_CBox FT_Outline_New FT_Outline_Translate FT_Set_Pixel_Sizes FT_Stroker_Done FT_Stroker_New FT_Stroker_Set -- Paul On Thu, Feb 11, 2010 at 9:17 PM, <[email protected]> wrote: > Thank you for report. Could you tell me the coverage > of your testing (font formats, what kind of tests are > executed, etc)? > > Regards, > mpsuzuki > > On Tue, 9 Feb 2010 13:55:37 -0800 > Paul Messmer <[email protected]> wrote: > > >Testing after removing the factor of 2 isn't showing any problems for me. > > > >-- Paul > > > >On Fri, Feb 5, 2010 at 3:48 PM, Paul Messmer <[email protected]> > wrote: > > > >> The overallocation is an urgent issue for me, but requires no urgent > action > >> on your part. The bar for me to make a small local fix to FreeType is > lower > >> than integrating an entirely new library release. After looking at the > code > >> further, I'm less concerned about the possibility of access of more than > >> n_points FT_Vectors, and I also have some testing I can exercise to give > me > >> some confidence the "fix" doesn't break anything... at least in the > scope of > >> my usage of FreeType. > >> > >> I wanted to bring this up with freetype-devel in order to get a second > >> opinion about the situation, and so the problem could be fixed in your > >> releases going forward. As there doesn't appear to be a good test suite > for > >> FreeType, your exercising caution with respect to the change is a fine > idea > >> as over-allocation can easily hide a latent and non-obvious bug. > >> > >> I'll keep you up to date on whether I encounter any failures after > removing > >> the factor of 2 in the allocation. Thanks. > >> > >> -- Paul > >> > >> On Thu, Feb 4, 2010 at 10:35 PM, <[email protected]> wrote: > >> > >>> Dear Paul, > >>> > >>> On Thu, 4 Feb 2010 15:29:28 -0800 > >>> Paul Messmer <[email protected]> wrote: > >>> >The code above seems to believe there are n_points FT_Vectors > allocated. > >>> > However, > >>> > > >>> >.../base/ftoutln.c: FT_Outline_New_Internal() > >>> > > >>> > if ( FT_NEW_ARRAY( anoutline->points, numPoints * 2L ) || > >>> > FT_NEW_ARRAY( anoutline->tags, numPoints ) || > >>> > FT_NEW_ARRAY( anoutline->contours, numContours ) ) > >>> > goto Fail; > >>> > anoutline->n_points = (FT_UShort)numPoints; > >>> > > >>> >This seems to be allocating 2*n_points FT_Vectors, so there's a > >>> difference > >>> >between how much memory is actually being used and how much it > believes > >>> is > >>> >being used. > >>> > >>> Great thank you for finding the problem. > >>> > >>> Tracking the history of the ftoutln.c, I think it came > >>> from freetype-1 era (!). A "point" in TT_Outline was > >>> described by TT_Vector, like this: > >>> > >>> > >>> > http://cvs.savannah.gnu.org/viewvc/freetype/lib/freetype.h?revision=1.72&root=freetype&view=markup > >>> > >>> struct TT_Outline_ > >>> { > >>> TT_Short n_contours; /* number of contours in glyph */ > >>> TT_UShort n_points; /* number of points in the glyph */ > >>> > >>> TT_Vector* points; /* the outline's points */ > >>> ... > >>> > >>> But TT_New_Outline() allocated the buffer as the twice > >>> of sizeof ( TT_F26Dot6 ), like this: > >>> > >>> > >>> > http://cvs.savannah.gnu.org/viewvc/freetype/lib/ttapi.c?revision=1.55&root=freetype&view=markup > >>> > >>> FT_EXPORT_FUNC( TT_Error ) > >>> TT_New_Outline( TT_UShort numPoints, > >>> TT_Short numContours, > >>> TT_Outline* outline ) > >>> { > >>> ... > >>> > >>> if ( ALLOC( outline->points, numPoints*2*sizeof ( TT_F26Dot6 ) ) > || > >>> ALLOC( outline->flags, numPoints *sizeof ( Byte ) ) > || > >>> ALLOC( outline->contours, numContours*sizeof ( UShort ) ) ) > >>> goto Fail; > >>> > >>> If it were written "sizeof ( FT_Vector )", this problem might not > >>> have occured. This "2" was carried over to FreeType2, because the > >>> initial version of FreeType2 still specified its size by twice of > >>> sizeof( FT_Pos ), like this: > >>> > >>> > >>> > http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?revision=1.1&root=freetype&view=markup > >>> > >>> if ( ALLOC_ARRAY( outline->points, numPoints * 2L, FT_Pos ) || > >>> ALLOC_ARRAY( outline->flags, numPoints, FT_Byte ) || > >>> ALLOC_ARRAY( outline->contours, numContours, FT_UShort ) ) > >>> goto Fail; > >>> > >>> Since freetype-2.1.0, FreeType2 uses FT_NEW_ARRAY() which > >>> automatically calculate the size of array element with > >>> sizeof( *first_arg ), so the factor 2 is not needed anymore, > >>> as you've pointed out. So, this problem has long life since > >>> freetype-2.1.0. The moment how overallocation can be found > >>> at: > >>> > >>> > >>> > http://cvs.savannah.gnu.org/viewvc/freetype2/src/base/ftoutln.c?root=freetype&r1=1.44&r2=1.45 > >>> > >>> -- > >>> > >>> I want to remove this extra factor "2". But you also mentioned > >>> the possibility that the buffer over outline->n_points is used > >>> in some special case, so I hesitate to remove it without careful > >>> check. This over allocation is urgent issue for you? If not, I > >>> want to work for this issue after next official release (expected > soon). > >>> > >>> Thank you again for detailed investigation and comment. > >>> > >>> Regards, > >>> mpsuzuki > >>> > >> > >> > > >
_______________________________________________ Freetype-devel mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/freetype-devel
