[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-2d-dev
Subject:    Re: [OpenJDK 2D-Dev] Double free bug in freetypeScaler.c
From:       Heikki Aitakangas <heikki.aitakangas () documill ! com>
Date:       2015-08-05 8:25:51
Message-ID: 55C1C88F.2070303 () documill ! com
[Download RAW message or body]

Thank you.

The bug has been in the code since at least Java 7, so affected versions 
should probably be 8,9 (or maybe 7,8,9)? Sorry I wasn't clear about that 
in my original message.


  -- Heikki Aitakangas

On 04/08/15 21:14, Phil Race wrote:
> 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
>
[prev in list] [next in list] [prev in thread] [next in thread]