freetype-commit
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[freetype2] master 13983b058 1/3: [base] Fix leak of internal stream mar


From: Werner Lemberg
Subject: [freetype2] master 13983b058 1/3: [base] Fix leak of internal stream marked external.
Date: Tue, 17 Jan 2023 03:01:20 -0500 (EST)

branch: master
commit 13983b058e840b144238e8c3f352a7e31ce53b86
Author: Ben Wagner <bungeman@chromium.org>
Commit: Werner Lemberg <wl@gnu.org>

    [base] Fix leak of internal stream marked external.
    
    `open_face_from_buffer` allocates a new `FT_Stream` to pass to
    `ft_open_face_internal`.  Because this is an `FT_OPEN_STREAM`,
    `ft_open_face_internal` will mark this as an 'external stream', which the
    caller must free.  However, `open_face_from_buffer` cannot directly free it
    because the stream must last as long as the face.  There is currently an
    attempt at this by clearing the 'external stream' bit after
    `open_face_from_buffer` returns successfully.  However, this is too late as
    the original stream may have already been closed and the stream on the face
    may not be the same stream as originally passed.
    
    It is tempting to use `FT_OPEN_MEMORY` and let `ft_open_face_internal`
    create the stream internally.  However, with this method there is no means
    to pass through a 'close' function to the created stream to free the
    underlying data, which must be owned by the stream.
    
    A possibility is to check on success if the stream of the face is the same
    as the original stream.  If it is then unset the external flag.  If not,
    then free the original stream.  Unfortunately, while no current
    implementation does so, it is possible that the face still has the original
    stream somewhere other than as the `FT_FaceRec::stream`.  The stream needs
    to remain available for the life of the face or until it is closed,
    whichever comes earlier.
    
    The approach taken here is to let the stream own itself.  When the stream is
    closed it will free itself.
    
    * src/base/ftobjs.c (memory_stream_close): Free `stream`.
    (open_face_from_buffer): Simplify error handling, since
    `ft_open_face_internal` always closes `args.stream` on any error.
    
    Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=54930
---
 src/base/ftobjs.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/src/base/ftobjs.c b/src/base/ftobjs.c
index 032b52600..7dcc7db58 100644
--- a/src/base/ftobjs.c
+++ b/src/base/ftobjs.c
@@ -1676,9 +1676,9 @@
 
 
     FT_FREE( stream->base );
-
     stream->size  = 0;
     stream->close = NULL;
+    FT_FREE( stream );
   }
 
 
@@ -1734,6 +1734,7 @@
     FT_Memory     memory = library->memory;
 
 
+    /* `memory_stream_close` also frees the stream object. */
     error = new_memory_stream( library,
                                base,
                                size,
@@ -1763,21 +1764,7 @@
       face_index &= 0x7FFF0000L; /* retain GX data */
 #endif
 
-    error = ft_open_face_internal( library, &args, face_index, aface, 0 );
-
-    if ( !error )
-      (*aface)->face_flags &= ~FT_FACE_FLAG_EXTERNAL_STREAM;
-    else
-#ifdef FT_MACINTOSH
-      FT_Stream_Free( stream, 0 );
-#else
-    {
-      FT_Stream_Close( stream );
-      FT_FREE( stream );
-    }
-#endif
-
-    return error;
+    return ft_open_face_internal( library, &args, face_index, aface, 0 );
   }
 
 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]