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
