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

[Heikki Aitakangas]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: FontDisposeTest.java
Type: text/x-java
Size: 2065 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20150804/4139e540/FontDisposeTest.java>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freetype-stream-free-fix.jdk8u.patch
Type: text/x-patch
Size: 2223 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20150804/4139e540/freetype-stream-free-fix.jdk8u.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freetype-stream-free-fix.jdk9.patch
Type: text/x-patch
Size: 2299 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20150804/4139e540/freetype-stream-free-fix.jdk9.patch>