Bug 102922

valgrind reports an invalid read error

==7173== Invalid write of size 8
==7173==    at 0x6C86FD7: cairo_truetype_font_write_glyf_table (cairo-truetype-subset.c:690)
==7173==    by 0x6C8858B: cairo_truetype_font_generate (cairo-truetype-subset.c:978)
==7173==    by 0x6C8858B: cairo_truetype_subset_init_internal (cairo-truetype-subset.c:1146)
==7173==    by 0x6CC637A: _cairo_pdf_surface_emit_truetype_font_subset (cairo-pdf-surface.c:5436)
==7173==    by 0x6CC637A: _cairo_pdf_surface_emit_unscaled_font_subset (cairo-pdf-surface.c:5910)
==7173==    by 0x6C84CE0: _cairo_sub_font_collect (cairo-scaled-font-subsets.c:746)
==7173==    by 0x6C84CE0: _cairo_scaled_font_subsets_foreach_internal (cairo-scaled-font-subsets.c:1067)
==7173==    by 0x6CC20D7: _cairo_pdf_surface_emit_font_subsets (cairo-pdf-surface.c:5956)
==7173==    by 0x6CC20D7: _cairo_pdf_surface_finish (cairo-pdf-surface.c:2031)
==7173==    by 0x6C68EC5: _cairo_surface_finish (cairo-surface.c:1033)
==7173==    by 0x6C69AD6: cairo_surface_finish (cairo-surface.c:1080)
==7173==    by 0x6C3DC8E: _cairo_paginated_surface_finish (cairo-paginated-surface.c:213)
==7173==    by 0x6C68EC5: _cairo_surface_finish (cairo-surface.c:1033)
==7173==    by 0x6C69AD6: cairo_surface_finish (cairo-surface.c:1080)
==7173==    by 0x5B0DB16: unix_end_run (gtkprintoperation-unix.c:373)
==7173==    by 0x59DF5F7: print_pages_idle (gtkprintoperation.c:2935)
==7173==    by 0x6097B8F: gdk_threads_dispatch (gdk.c:743)
==7173==    by 0x776ADE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x776B1AF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x776B23B: g_main_context_iteration (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x6FAEBEC: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.5400.0)
==7173==    by 0x127C97: main (main.c:316)
==7173==  Address 0x166d0558 is 8 bytes after a block of size 160 alloc'd
==7173==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7173==    by 0x6C87FAF: _cairo_truetype_font_create (cairo-truetype-subset.c:205)
==7173==    by 0x6C87FAF: cairo_truetype_subset_init_internal (cairo-truetype-subset.c:1134)
==7173==    by 0x6CC637A: _cairo_pdf_surface_emit_truetype_font_subset (cairo-pdf-surface.c:5436)
==7173==    by 0x6CC637A: _cairo_pdf_surface_emit_unscaled_font_subset (cairo-pdf-surface.c:5910)
==7173==    by 0x6C84CE0: _cairo_sub_font_collect (cairo-scaled-font-subsets.c:746)
==7173==    by 0x6C84CE0: _cairo_scaled_font_subsets_foreach_internal (cairo-scaled-font-subsets.c:1067)
==7173==    by 0x6CC20D7: _cairo_pdf_surface_emit_font_subsets (cairo-pdf-surface.c:5956)
==7173==    by 0x6CC20D7: _cairo_pdf_surface_finish (cairo-pdf-surface.c:2031)
==7173==    by 0x6C68EC5: _cairo_surface_finish (cairo-surface.c:1033)
==7173==    by 0x6C69AD6: cairo_surface_finish (cairo-surface.c:1080)
==7173==    by 0x6C3DC8E: _cairo_paginated_surface_finish (cairo-paginated-surface.c:213)
==7173==    by 0x6C68EC5: _cairo_surface_finish (cairo-surface.c:1033)
==7173==    by 0x6C69AD6: cairo_surface_finish (cairo-surface.c:1080)
==7173==    by 0x5B0DB16: unix_end_run (gtkprintoperation-unix.c:373)
==7173==    by 0x59DF5F7: print_pages_idle (gtkprintoperation.c:2935)
==7173==    by 0x6097B8F: gdk_threads_dispatch (gdk.c:743)
==7173==    by 0x776ADE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x776B1AF: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x776B23B: g_main_context_iteration (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.5400.0)
==7173==    by 0x6FAEBEC: g_application_run (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.5400.0)
==7173==    by 0x127C97: main (main.c:316)

let me know if you need more debug info

It's crashing trying to free a font:

    free (font->glyphs);

So would probably be useful to know what fonts are being used in the document.  Perhaps the reporter could construct a PoC using the same fonts as in the crashing doc and see if they crash that one too.

I don't have the source of the document and the fonts with which it's been built but according to the content of the pdf it references the following fonts:

ComicSansMS,Bold
Helvetica
Helvetica-Bold
Helvetica-Oblique
MSShellDlg,Bold

> ==7173== Invalid write of size 8
> ==7173==    at 0x6C86FD7: cairo_truetype_font_write_glyf_table (cairo-truetype-subset.c:690)
> ==7173==    by 0x6C8858B: cairo_truetype_font_generate (cairo-truetype-subset.c:978)
> ==7173==    by 0x6C8858B: cairo_truetype_subset_init_internal (cairo-truetype-subset.c:1146)
[...]
> ==7173==  Address 0x166d0558 is 8 bytes after a block of size 160 alloc'd
> ==7173==    at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==7173==    by 0x6C87FAF: _cairo_truetype_font_create (cairo-truetype-subset.c:205)
> ==7173==    by 0x6C87FAF: cairo_truetype_subset_init_internal (cairo-truetype-subset.c:1134)


Line 690 is

> font->glyphs[i].location = next - start_offset;

The only statement modifying variable i is:

> for (i = 0; i < font->base.num_glyphs; i++) {

Since the above line is outside of this for-loop, i is at most font->base.num_glyphs.

Looking a bit more at the callers, init_internal allocates the font with _cairo_truetype_font_create(), which does:

> font->glyphs = calloc (font->num_glyphs_in_face + 1, sizeof (subset_glyph_t));

(I think) "num_glyphs_in_face" is (supposed to be) the number of glyphs in the original font and "base.num_glyphs" is the number of glyphs which ended up in the subsetted font. No idea why this manages to subset more glyphs than the font even has. There are more numbers involved here (cairo_truetype_subset_init_internal() uses font->scaled_font_subset->num_glyphs...).

Behdad, ideas?

@Sebastian: You could try adding assert(font->base.num_glyphs < font->num_glyphs_in_face); before the end of cairo_truetype_font_use_glyph(). This assert should trigger earlier than what valgrind can detect (ignore the error which is only detected during free(), that's longer after things went wrong). Hopefully the backtrace for this one would be more useful, but I am just guessing here.

> Hopefully the backtrace for this one would be more useful, but I am just guessing here.

backtrace is

#1  0x00007ffff48bef5d in __GI_abort () at abort.c:90
#2  0x00007ffff48b4f17 in __assert_fail_base (fmt=<optimised out>, assertion=assertion@entry=0x7ffff5de0e18 "font->base.num_glyphs < font->num_glyphs_in_face", file=file@entry=0x7ffff5de0e00 "cairo-truetype-subset.c", line=line@entry=1029, function=function@entry=0x7ffff5de0e80 <__PRETTY_FUNCTION__.11613> "cairo_truetype_font_use_glyph") at assert.c:92
#3  0x00007ffff48b4fc2 in __GI___assert_fail (assertion=assertion@entry=0x7ffff5de0e18 "font->base.num_glyphs < font->num_glyphs_in_face", file=file@entry=0x7ffff5de0e00 "cairo-truetype-subset.c", line=line@entry=1029, function=function@entry=0x7ffff5de0e80 <__PRETTY_FUNCTION__.11613> "cairo_truetype_font_use_glyph")
    at assert.c:101
#4  0x00007ffff5d8da79 in cairo_truetype_font_use_glyph (font=<optimised out>, glyph=<optimised out>, out=<optimised out>) at cairo-truetype-subset.c:1029
#5  0x00007ffff5d8f63f in cairo_truetype_subset_init_internal (truetype_subset=truetype_subset@entry=0x7fffffffd840, font_subset=font_subset@entry=0x7fffffffd9b0, is_pdf=is_pdf@entry=1) at cairo-truetype-subset.c:1142
#6  0x00007ffff5d8ffea in _cairo_truetype_subset_init_pdf (truetype_subset=truetype_subset@entry=0x7fffffffd840, font_subset=font_subset@entry=0x7fffffffd9b0)
    at cairo-truetype-subset.c:1244
#7  0x00007ffff5dceb8b in _cairo_pdf_surface_emit_truetype_font_subset (font_subset=0x7fffffffd9b0, surface=0x55555621aad0) at cairo-pdf-surface.c:5436
#8  0x00007ffff5dceb8b in _cairo_pdf_surface_emit_unscaled_font_subset (font_subset=0x7fffffffd9b0, closure=0x55555621aad0) at cairo-pdf-surface.c:5910
#9  0x00007ffff5d8c20e in _cairo_sub_font_collect (closure=0x7fffffffd960, entry=0x55555605bc00) at cairo-scaled-font-subsets.c:746
#10 0x00007ffff5d8c20e in _cairo_scaled_font_subsets_foreach_internal (font_subsets=<optimised out>, font_subset_callback=font_subset_callback@entry=0x7ffff5dceaa0 <_cairo_pdf_surface_emit_unscaled_font_subset>, closure=closure@entry=0x55555621aad0, type=type@entry=CAIRO_SUBSETS_FOREACH_UNSCALED)
    at cairo-scaled-font-subsets.c:1067
#11 0x00007ffff5d8d0b7 in _cairo_scaled_font_subsets_foreach_unscaled (font_subsets=<optimised out>, font_subset_callback=font_subset_callback@entry=0x7ffff5dceaa0 <_cairo_pdf_surface_emit_unscaled_font_subset>, closure=closure@entry=0x55555621aad0) at cairo-scaled-font-subsets.c:1095
#12 0x00007ffff5dca673 in _cairo_pdf_surface_emit_font_subsets (surface=0x55555621aad0) at cairo-pdf-surface.c:5956
#13 0x00007ffff5dca673 in _cairo_pdf_surface_finish (abstract_surface=0x55555621aad0) at cairo-pdf-surface.c:2031
#14 0x00007ffff5d70486 in _cairo_surface_finish (surface=0x55555621aad0)
    at cairo-surface.c:1033
#15 0x00007ffff5d710b7 in INT_cairo_surface_finish (surface=0x55555621aad0)
    at cairo-surface.c:1080
#16 0x00007ffff5d43365 in _cairo_paginated_surface_finish (abstract_surface=0x555556197e90) at cairo-paginated-surface.c:213
#17 0x00007ffff5d70486 in _cairo_surface_finish (surface=0x555556197e90)
    at cairo-surface.c:1033
#18 0x00007ffff5d710b7 in INT_cairo_surface_finish (surface=0x555556197e90)
    at cairo-surface.c:1080
#19 0x00007ffff6f5fb17 in unix_end_run (op=0x55555583a430, wait=0, cancelled=0)
    at ././gtk/gtkprintoperation-unix.c:373
#20 0x00007ffff6e315f8 in print_pages_idle (user_data=0x555555f9f840)
    at ././gtk/gtkprintoperation.c:2935
#21 0x00007ffff68ceb90 in gdk_threads_dispatch (data=0x5555559cb820)
    at ././gdk/gdk.c:743
#22 0x00007ffff5225de5 in g_main_context_dispatch ()
    at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#23 0x00007ffff52261b0 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0

The font is broken in some way. The backtrace doesn't help because the problem occurred earlier when it was parsing some data out of the font. There is not much I can do without the pdf.

The issue was not there in .6, bisecting the commits it started with 
"scaled-font-subsets: if glyph 0 used for rendering, remap to different index"

https://cgit.freedesktop.org/cairo/commit/?id=52cbf42b74785c3c3c2d15effe7bdb416ff9c8b2

could that fix be backported to the 1.14 serie?

Patch should work with 1.14.

right, I backported it to Ubuntu but I was wondering if it would make sense to be commited upstream to the stable serie as well

If there is another 1.14 release it will be included.