[OpenJDK 2D-Dev] Double free bug in freetypeScaler.c

[Phil Race]

Yes, this is the right place. The way to submit patches is a bit more 
fuzzy but
I think it'll be fine for this one.  I reproduced the crash using your test.
I've filed https://bugs.openjdk.java.net/browse/JDK-8132985 to track this.

-phil.

On 08/04/2015 07:46 AM, Heikki Aitakangas wrote:
> Greetings,
>
> The freeNativeResources function in freetypeScaler.c frees a pointer 
> obtained from FreeType internals:
>
>>     stream = scalerInfo->face->stream;
>>
>>     FT_Done_Face(scalerInfo->face);
>>
>> ...snip...
>>
>>    if (stream != NULL) {
>>         free(stream);
>>    }
>
> This is done in order to free a FT_StreamRec that Java allocates in 
> some cases when initialising the native scaler. However, the current 
> approach is wrong because:
>
> - FT_Done_Face will itself also free the face->stream pointer, if
>   that memory was allocated by FreeType itself instead of by Java.
> - FT_FaceRec.stream is explicitly noted to be a private field in
>   FreeType documentation.
>
> With the way Java uses FreeType, FT ends up allocating the 
> FT_StreamRec structure in Java's Type 1 font case. As a result we've 
> been observing JVM crashes due to a double free when:
> - a java.awt.Font is constructed from Type 1 font data
> - it becomes garbage
> - the associated FreetypeFontScaler is disposed
>
> The attached FontDisposeTest.java can demonstrate the crash. Pass the 
> path to a Type 1 font file as argument and it will load the font and 
> force native scaler disposal. If memory allocation debugging is not 
> active by default, it can be forced on by setting an environment 
> variable 'MALLOC_CHECK_=3' (glibc, for Windows see PageHeap).
> A Type 1 font file can be found from Ghostscript 
> (ftp://ftp.gnu.org/gnu/ghostscript/gnu-gs-fonts-other-6.0.tar.gz)
>
>
> FreeType records whether the pointer in FT_FaceRec.stream was 
> allocated externally or by itself in the FT_FaceRec.face_flags field. 
> However the documentation for FT_FACE_FLAG_EXTERNAL_STREAM says
>
>> Used internally by FreeType to indicate that a face's stream was 
>> provided by the client application and should not be destroyed when 
>> FT_Done_Face is called. Don't read or test this flag.
>
> Therefore Java should maintain it's own copy of the stream pointer so 
> it can know with certainty what memory it needs to free, if any.
>
> Attached are patches to fix the bug in jdk8u and jdk9. I've added a 
> field `faceStream` to the FTScalerInfo struct to keep track of the 
> Java-allocated stream.
>
>
>
> PS. if this is not the correct mailing list, or a valid way of 
> submitting a patch, could you point me to the correct one?
>
>
>  -- Heikki Aitakangas