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

Reply via email to