bugThe FreeType Project - Bugs: bug #46480, Leak of glyph_name after being...

 
 

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

bug #46480: Leak of glyph_name after being allocated in bdf parser. (Part II)

Submitter:  bungeman <bungeman>
Submitted:  Thu 19 Nov 2015 09:02:11 PM UTC
   
 
Severity:  3 - Normal Item Group:  Incorrect behaviour
Status:  Fixed Privacy:  Public
Assigned to:  wl Open/Closed:  Closed
Planned Release:  2.6.2

Jump to the original submission

Wed 25 Nov 2015 06:54:41 AM UTC, comment #6: 

Your analysis is correct, thanks.  This is now fixed in git.

Werner LEMBERG <wl>
Group administrator
Tue 24 Nov 2015 09:37:02 PM UTC, comment #5: 

Taking a quick look at the 'a650' example (the first address with a * in the previous comment) it looks like it's possible to create the glyph_name but then never assign it to a glyph but at the same time never produce an error.

It appears that the intent is that p->glyph_name either holds the next glyph's name or NULL, and any time a glyph's name is set from p->glyph_name, p->glyph_name is cleared to NULL (the name is moved to the glyph from the parser state).

As a result, it seems like the 'FT_FREE( p->glyph_name );' could just be moved from the 'Fail' block to the 'Exit' block in bdf_load_font to work around this, since it appears that in the 'Exit' case it is assumed that it is already NULL. On the other hand, it may be an error to get to the 'Exit' block and p->glyph_name not to be NULL, in which case some other part of the state machine would need to make that an error in this case (so that it would go through the 'Fail' block).

bungeman <bungeman>
Tue 24 Nov 2015 09:06:33 PM UTC, comment #4: 

I ran this in gdb with LSAN_OPTIONS="report_objects=1"

(gdb) break bdflib.c:1683
(gdb) run crash-d704d97b3f9c02c4231a1ceba71f266a7ea7c85e

(gdb) print p->glyph_name
(gdb) cont

with these last two commands repeated until completion. This produced the following output (annotated for clarity):

0x60200000a770
0x60200000a750
0x60200000a710
0x60200000a6d0
0x60200000a690
0x60200000a650 *
0x602000005d90
0x602000005d70
0x602000005d30
0x602000005cf0
0x602000005cb0
0x602000005c70 *
0x6020000013b0
0x602000001390
0x602000001350
0x602000001310
0x6020000012d0
0x602000001290 *

Objects leaked above:
0x602000005c70 (2 bytes)
0x60200000a650 (2 bytes)
0x602000001290 (2 bytes)

Hopefully this helps narrow things down.

bungeman <bungeman>
Mon 23 Nov 2015 06:05:43 PM UTC, comment #3: 

In the event that it helps, here are two more examples which appear to trigger the leak.

Reviewing my previous comment I realize it reads rather ambiguous. By 'issue' in 'This isn't an issue with FreeType' I meant the issue that it's difficult to re-run a libFuzzer run.

(file #35531, file #35532)

bungeman <bungeman>
Mon 23 Nov 2015 03:40:53 PM UTC, comment #2: 

Attached is a reproducing case. This took about three days of running with libFuzzer to find again (with 20 jobs and 20 workers) with the 'lsan each time' patch.

This isn't an issue with FreeType, but for cases like this there needs to be a simple and documented way to exactly re-run a libFuzzer run (same inputs and seeds). Otherwise eventually an issue like this will be found where it becomes prohibitively expensive to extract a test case due to the fact that creating such cases is probabilistic. I'll see about opening an issue with libFuzzer about this.

(file #35530)

bungeman <bungeman>
Fri 20 Nov 2015 05:03:16 PM UTC, comment #1: 

OK.

Werner LEMBERG <wl>
Group administrator
Thu 19 Nov 2015 09:02:11 PM UTC, original submission:  

The fix for 'Leak of glyph_name after being allocated in bdf parser.' did fix that test case, but now I'm seeing this leak by some other means. This is with FreeType 68fb4789a582561606fafbe51e7a217598bb35ec . The leak is reported as

ERROR: LeakSanitizer: detected memory leaks
Direct leak of 10 byte(s) in 5 object(s) allocated from:
    #0 0x4af47b in __interceptor_malloc llvm_all/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:40:3
    #1 0x545089 in ft_mem_qalloc freetype2/src/base/ftutil.c:76:15
    #2 0x545089 in ft_mem_alloc freetype2/src/base/ftutil.c:55
    #3 0x545089 in ft_mem_qrealloc freetype2/src/base/ftutil.c:146
    #4 0x4fe3e9 in ft_mem_realloc freetype2/src/base/ftutil.c:102:13
    #5 0x6ad6d2 in _bdf_parse_glyphs freetype2/src/bdf/bdflib.c:1680:12
    #6 0x68c4ca in _bdf_readstream freetype2/src/bdf/bdflib.c:790:17
    #7 0x68c4ca in bdf_load_font freetype2/src/bdf/bdflib.c:2474
    #8 0x68c4ca in BDF_Face_Init freetype2/src/bdf/bdfdrivr.c:364
    #9 0x50a278 in open_face freetype2/src/base/ftobjs.c:1178:15
    #10 0x5076ca in FT_Open_Face freetype2/src/base/ftobjs.c:2176:19
    #11 0x509a1c in FT_New_Memory_Face freetype2/src/base/ftobjs.c:1268:12
    #12 0x4da1f3 in run_test(unsigned char const*, unsigned long) freetype2/src/tools/ftfuzzer/ftfuzzer.cc:234:10
    #13 0x4d9ee1 in LLVMFuzzerTestOneInput freetype2/src/tools/ftfuzzer/ftfuzzer.cc:352:18

However, this leak appears to be in already covered code and rather rare to reproduce, as it seems to have happened three times in 7151847 fuzz loops. This also happened the previous time I ran with the fuzzer normally, so it appears to be real. I did let it run overnight with the 'lsan each iteration' patch, but it hasn't happened yet.  I'll see if I can create a reproduction by focusing on bdf, but it may take some time.

bungeman <bungeman>

 

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

Attached Files

 

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 bungeman (Submitted the item)
  •  

    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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-11-25 wl Open/ClosedOpen Closed
        Planned ReleaseNone 2.6.2
    2015-11-25 wl StatusNeed Info Fixed
    2015-11-23 bungeman Attached File- Added crash-607fd157d2cd72bdc5b6df6f6e1eeaf61bb55657, #35531
        Attached File- Added crash-d704d97b3f9c02c4231a1ceba71f266a7ea7c85e, #35532
    2015-11-23 bungeman Attached File- Added crash-6519982d4d8593656c504a52313ab5fd86b22281, #35530
    2015-11-20 wl StatusNone Need Info
        Assigned toNone wl

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code