Suggested patch for memory leak upon allocation error...
Migrated from: [SAVANNAH-58609]
Sebastian Rasmussen reported:
I have come across a location where FreeType 2.10.0 fails to free all resources in case an allocation fails.
This was found using gcc 9.3.0 ASAN and a custom allocator that lets me control when/where it will return NULL.
This is the gdb backtrace (relevant for Freetype) where the allocator returns NULL:
#1 (closed) 0x0000555555b1c22d in ft_mem_qalloc (memory=0x621000002920, size=3871,
p_error=0x7fffffff8ce0) at freetype/src/base/ftutil.c:76
#2 (closed) 0x0000555555b1c0b2 in ft_mem_alloc (memory=0x621000002920, size=3871,
p_error=0x7fffffff8dc0) at freetype/src/base/ftutil.c:55
#3 (closed) 0x0000555555b3981b in cff_index_get_pointers (idx=0x7fffffff8eb0,
table=0x622000000760, pool=0x622000000768, pool_size=0x622000000770)
at freetype/src/cff/cffload.c:432
#4 (closed) 0x0000555555b43b13 in cff_font_load (library=0x614000000450,
stream=0x6080000003b0, face_index=0, font=0x622000000110,
face=0x61a000000c90, pure_cff=1 '\001', cff2=0 '\000')
at freetype/src/cff/cffload.c:2319
#5 (closed) 0x0000555555b48112 in cff_face_init (stream=0x6080000003b0,
cffface=0x61a000000c90, face_index=0, num_params=0, params=0x0)
at freetype/src/cff/cffobjs.c:615
#6 (closed) 0x0000555555b01593 in open_face (driver=0x60c0000001d0,
astream=0x7fffffff9340, external_stream=0 '\000', face_index=0,
num_params=0, params=0x0, aface=0x7fffffff9360)
at freetype/src/base/ftobjs.c:1403
#7 (closed) 0x0000555555b023a3 in ft_open_face_internal (library=0x614000000450,
args=0x7fffffff9420, face_index=0, aface=0x7fffffff9530,
test_mac_fonts=1 '\001') at freetype/src/base/ftobjs.c:2475
#8 (closed) 0x0000555555b01bb2 in FT_New_Memory_Face (library=0x614000000450,
file_base=0x55555668c7db "\001", file_size=34024, face_index=0,
aface=0x7fffffff9530) at freetype/src/base/ftobjs.c:1493
This is the corresponding backtrace where the allocation of the data that leaks occurs:
#1 (closed) 0x5585c1da75a4 in ft_mem_qrealloc freetype/src/base/ftutil.c:146
#2 (closed) 0x5585c1da7387 in ft_mem_realloc freetype/src/base/ftutil.c:102
#3 (closed) 0x5585c1dc47bb in cff_index_get_pointers freetype/src/cff/cffload.c:431
#4 (closed) 0x5585c1dceb12 in cff_font_load freetype/src/cff/cffload.c:2319
#5 (closed) 0x5585c1dd3111 in cff_face_init freetype/src/cff/cffobjs.c:615
#6 (closed) 0x5585c1d8c592 in open_face freetype/src/base/ftobjs.c:1403
#7 (closed) 0x5585c1d8d3a2 in ft_open_face_internal freetype/src/base/ftobjs.c:2475
#8 (closed) 0x5585c1d8cbb1 in FT_New_Memory_Face freetype/src/base/ftobjs.c:1493
This points to cff_index_get_pointers() allocating something that is not correctly freed upon error.
Looking at the source code (https://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/cff/cffload.c#n429)...
if ( idx->count > 0 &&
!FT_NEW_ARRAY( t, idx->count + 1 ) &&
( !pool || !FT_ALLOC( new_bytes, new_size ) ) )
/* ... */
Exit:
return error,
}
... reveals that if FT_NEW_ARRAY() succeeds and pool is non-NULL and FT_ALLOC() fails in this
part of the code, then nothing is cleaned up at the end of the function. This is true for
FreeType 2.10.0 as well as for git HEAD. An example of where FreeType handles this situation
better is in cid_read_subrs() (https://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/cid/cidload.c#n592):
if ( FT_NEW_ARRAY( subr->code, num_subrs + 1 ) ||
FT_ALLOC( subr->code[0], data_len ) )
goto Fail,
/* ... */
Exit:
FT_FREE( offsets ),
return error,
Fail:
if ( face->subrs )
{
for ( n = 0, n < cid->num_dicts, n++ )
{
if ( face->subrs[n].code )
FT_FREE( face->subrs[n].code[0] ),
FT_FREE( face->subrs[n].code ),
}
FT_FREE( face->subrs ),
}
goto Exit,
}
Here face->subrs.code is freed even if FT_ALLOC() fails. I contemplated rearranging
cff_index_get_pointers() similarly, but ended up doing error handling similar to other
locations in cffload.c, e.g. https://git.savannah.gnu.org/cgit/freetype/freetype2.git/tree/src/cff/cffload.c#n1076
I will shortly attach my proposed patch fixing this memory leak.