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

 
 

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

Submitted by:  bungeman <bungeman>
Submitted on:  Thu 19 Nov 2015 09:02:11 PM UTC  
 
Severity: 3 - NormalItem Group: Incorrect behaviour
Status: FixedPrivacy: Public
Assigned to: Werner LEMBERG <wl>Open/Closed: Closed
Planned Release: 2.6.2

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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.)

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by wl (Posted a comment)
  • -unavailable- added by bungeman (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 25 Nov 2015 06:54:41 AM UTCwlOpen/ClosedOpen=>Closed
      Planned ReleaseNone=>2.6.2
    Wed 25 Nov 2015 06:54:40 AM UTCwlStatusNeed Info=>Fixed
    Mon 23 Nov 2015 06:05:43 PM UTCbungemanAttached File-=>Added crash-607fd157d2cd72bdc5b6df6f6e1eeaf61bb55657, #35531
      Attached File-=>Added crash-d704d97b3f9c02c4231a1ceba71f266a7ea7c85e, #35532
    Mon 23 Nov 2015 03:40:53 PM UTCbungemanAttached File-=>Added crash-6519982d4d8593656c504a52313ab5fd86b22281, #35530
    Fri 20 Nov 2015 05:03:16 PM UTCwlStatusNone=>Need Info
      Assigned toNone=>wl

    Back to the top


    Powered by Savane 3.1-cleanup1