bugThe FreeType Project - Bugs: bug #58416, Woff2 TTCs memory leak

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #58416: Woff2 TTCs memory leak

Submitter:  None
Submitted:  Wed 20 May 2020 07:16:38 PM UTC
   
 
Severity:  3 - Normal Item Group:  Crash
Status:  Need Info Privacy:  Public
Assigned to:  wl Open/Closed:  Open
Planned Release:  None

Fri 05 Jun 2020 04:41:03 AM UTC, comment #5: 

I think the problem is not with `FT_Open_Face` but that the WOFF2 font itself if generated as you describe doesn't trigger an error at all – at least using the program and font versions mentioned earlier.  A 'Bad Format' font error should be reproducible regardless how the data is stored.

The stream object pointed to by the stack trace is internal to FreeType; it is used to communicate with the Brotli library.  In other words, a FreeType demo program like `ftview` should expose the problem too, AFAICS, but it doesn't.

Can you make the exact file you are using for testing temporarily available for download?

Werner LEMBERG <wl>
Group administrator
Thu 04 Jun 2020 02:50:54 AM UTC, comment #4: 

Here's the stack trace from ASAN:

Direct leak of 80 byte(s) in 1 object(s) allocated from:
    #0 0x311c9fd in malloc /b/swarming/w/ir/cache/builder/src/third_party/llvm/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
    #1 0x3c1d65e in SbMemoryAllocateChecked memory.cc:127:19
    #2 0x3c1d65e in (anonymous namespace)::SbMemoryAllocateImpl(unsigned long) memory.cc:191
    #3 0x3c1d65e in SbMemoryAllocate memory.cc:69
    #4 0x9284eb4 in sk_malloc_flags(unsigned long, unsigned int) SkMemory_starboard.cc
    #5 0x9cd8bcf in ft_mem_qalloc freetype2/src/base/ftutil.c:76:15
    #6 0x9cd8bcf in ft_mem_alloc freetype2/src/base/ftutil.c:55
    #7 0x9dc5c0a in woff2_open_font freetype2/src/sfnt/sfwoff2.c:2165:10
    #8 0x9dc5c0a in sfnt_open_font freetype2/src/sfnt/sfobjs.c:398
    #9 0x9db229c in sfnt_init_face freetype2/src/sfnt/sfobjs.c:540:13
    #10 0x9de8610 in tt_face_init freetype2/src/truetype/ttobjs.c:636:13
    #11 0x9cf7e2f in open_face freetype2/src/base/ftobjs.c:1403:15
    #12 0x9ce04c2 in ft_open_face_internal freetype2/src/base/ftobjs.c:2475:19

Avery Crits-Christoph <averycc>
Thu 04 Jun 2020 02:34:08 AM UTC, comment #3: 

Hey, thanks for the response and sorry for the delay in responding. You're right, that demo isn't triggering the error like I thought it would. Do you have any demos that use FT_Open_Face? The error only occurs when FT_Open_Face is used to provide an external stream.

Also, my original message was cut off a bit. I meant to add that this patch is good for illustrating the problem but is probably not a full fix by itself.

Avery Crits-Christoph <averycc>
Tue 26 May 2020 05:58:59 AM UTC, comment #2: 

Thanks for the patch; will have a look soon.  However, I tried to reproduce the issue and wasn't able to.  Neither valgrind nor ASAN show a problem – no error was triggered at all.

Could you describe in more detail which error you get?  I used FreeType's git master (`make devel; make`), together with `NotoSansCJK-Regular.ttc` version 2.001 that gets processed with `woff2_compress` from woff2's git master.

Werner LEMBERG <wl>
Group administrator
Wed 20 May 2020 07:23:20 PM UTC, comment #1: 

Hi, sorry I forgot to log in. I submitted this (averycc).

Uploading files here isn't working, so here is how to get NotoSansCJK-Regular.woff2:

1. Download NotoSansCJK-Regular.ttc at https://www.google.com/get/noto/help/cjk/
2. Run "sudo apt install woff2" to download woff2 tools.
3. Run "woff2_compress NotoSansCJK-Regular.ttc"

Also here is the suggested fix diff:

diff --git a/src/base/ftobjs.c b/src/base/ftobjs.c
index 1b04261..9f5ff3e 100644
--- a/src/base/ftobjs.c
+++ b/src/base/ftobjs.c
@@ -1422,8 +1422,6 @@
       goto Fail;
     }
 
-    *aface = face;
-
   Fail:
     if ( error )
     {
@@ -1431,10 +1429,9 @@
       if ( clazz->done_face )
         clazz->done_face( face );
       FT_FREE( internal );
-      FT_FREE( face );
-      *aface = NULL;
     }
 
+    *aface = face;
     return error;
   }
 
@@ -2477,6 +2474,10 @@
           if ( !error )
             goto Success;
 
+          external_stream = (face->face_flags & FT_FACE_FLAG_EXTERNAL_STREAM) != 0;
+          FT_FREE(face);
+          face = NULL;
+
 #ifdef FT_CONFIG_OPTION_MAC_FONTS
           if ( test_mac_fonts                                           &&
                ft_strcmp( cur[0]->clazz->module_name, "truetype" ) == 0 &&

Avery Crits-Christoph <averycc>
Wed 20 May 2020 07:16:38 PM UTC, original submission:  

Hey Freetype team,

I've come across a memory leak in your new woff2 support (2.10.2) and have a repro case and suggested fix. It's a little hard to see the leak in the repro because ./ftdemos doesn't report it (we see it with ASAN), but I'm not sure if you have a way of turning that on.

To reproduce:
11. Configure/make freetype with --with-brotli=yes
2. Make ftdemos
3. ./ftview 16 NotoSansCJK-Regular.woff2

(Note, this only seems to happen for TTC woff2 fonts since these trigger Bad Format errors, which leads to the failure flow.)

You're updating the external stream flag bit here when allocating a new woff2 stream:
https://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/sfnt/sfwoff2.c#n2266

But when freeing the stream upon an error, you don't use the update

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by podtelez (Updated the item)
  • -email is unavailable- added by podtelez
  • -email is unavailable- added by averycc (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2020-06-15 wl StatusNone Need Info
    2020-05-26 wl Assigned todxdy wl
    2020-05-22 podtelez Assigned toNone dxdy
        Carbon-Copy- Added dxdy

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code