mono / libgdiplus

C-based implementation of the GDI+ API
http://www.mono-project.com/
MIT License
329 stars 171 forks source link

Memory corruption due to ellipsed text when using Pango #722

Open handerss-spotfire opened 2 years ago

handerss-spotfire commented 2 years ago

Reproducible example: https://github.com/handerss-tibco/libgdiplus-pango-memory-corruption

There are memory corruption issues when performing MeasureString on multiple threads if measurement uses ellipsed text. This corruption error only occurs when libgdiplus is compiled with pango as the backend. Here's a minimal reproducible example:

using System.Drawing;
using System.Threading.Tasks;
using System.Collections.Generic;

var tasks = new List<Task>();

for (var i = 0; i < 1000; i++)
{
    tasks.Add(Task.Run(() =>
    {
        using (var bitmap = new Bitmap(100, 100))
        using (var g = Graphics.FromImage(bitmap))
        using (var font = new Font("Arial", 10))
        using (var f = new StringFormat())

        {
            f.Trimming = StringTrimming.EllipsisCharacter; // If this line is commented out, there will be no glib warnings.
            g.MeasureString("APA", font, new PointF(0, 0), f);
        }
    }));

}

Task.WaitAll(tasks.ToArray());

This program produces a number of warnings/criticals:

(process:93): GLib-WARNING **: 11:49:32.003: corrupted double-linked list detected

(process:93): GLib-CRITICAL **: 11:49:32.003: g_queue_push_head_link: assertion 'link->next == NULL' failed

These issues are only observed when multiple threads are used, and when libgdiplus is compiled with pango backend. We also observe these issues when compiling libgdiplus from main and using a new version of Pango 1.46.2 (Debian 11).

handerss-spotfire commented 2 years ago

Using valgrind we managed to track this issue down to thread-unsafe usage of font caches. Pango is thread safe in this regard by keeping a font cache per thread. Libgdiplus however sends a global font cache to Pango resulting in use after free issues.

This case can be fixed by replacing the usage of family->collection->pango_font_map with pango_cairo_font_map_get_default().

Here's an example of the valgrind output:

==431== Thread 209:
==431== Invalid read of size 8
==431==    at 0x3305644B: pango_fc_fontset_key_equal (pangofc-fontmap.c:432)
==431==    by 0x2F20DE02: g_hash_table_lookup (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6600.8)
==431==    by 0x3305946C: pango_fc_font_map_load_fontset (pangofc-fontmap.c:2200)
==431==    by 0x33056C8A: pango_fc_font_map_load_font (pangofc-fontmap.c:2135)
==431==    by 0x2E13E3AD: pango_layout_get_empty_extents_at_index.part.0 (pango-layout.c:4929)
==431==    by 0x2E142265: pango_layout_get_empty_extents_at_index (pango-layout.c:4879)
==431==    by 0x2E142265: pango_layout_check_lines.part.0 (pango-layout.c:4326)
==431==    by 0x2E144E81: _pango_layout_get_iter (pango-layout.c:6252)
==431==    by 0x2E144FA8: pango_layout_get_iter (pango-layout.c:6237)
==431==    by 0x2E0D5478: pango_MeasureString (text-pango.c:617)
==431==    by 0x5C4B5803: ???
==431==    by 0x5C4B5657: ???
==431==    by 0x5C4A83D6: ???
==431==  Address 0xbbef0378 is 8 bytes inside a block of size 104 free'd
==431==    at 0x48399AB: free (vg_replace_malloc.c:538)
==431==    by 0x33056B89: pango_fc_fontset_key_free (pangofc-fontmap.c:484)
==431==    by 0x33056B89: pango_fc_fontset_finalize (pangofc-fontmap.c:1219)
==431==    by 0x2E18411D: g_object_unref (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.6600.8)
==431==    by 0x2F20D340: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6600.8)
==431==    by 0x2F20E1E7: g_hash_table_insert (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6600.8)
==431==    by 0x33059951: pango_fc_font_map_load_fontset (pangofc-fontmap.c:2210)
==431==    by 0x33056C8A: pango_fc_font_map_load_font (pangofc-fontmap.c:2135)
==431==    by 0x2E13E3AD: pango_layout_get_empty_extents_at_index.part.0 (pango-layout.c:4929)
==431==    by 0x2E142265: pango_layout_get_empty_extents_at_index (pango-layout.c:4879)
==431==    by 0x2E142265: pango_layout_check_lines.part.0 (pango-layout.c:4326)
==431==    by 0x2E144368: pango_layout_get_extents_internal (pango-layout.c:2685)
==431==    by 0x2E144802: pango_layout_get_pixel_extents (pango-layout.c:2903)
==431==    by 0x2E0D4B02: gdip_pango_setup_layout (text-pango.c:457)
==431==  Block was alloc'd at
==431==    at 0x483877F: malloc (vg_replace_malloc.c:307)
==431==    by 0x2F226D48: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6600.8)
==431==    by 0x2F23ED7D: g_slice_alloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.6600.8)
==431==    by 0x330598A2: pango_fc_fontset_key_copy (pangofc-fontmap.c:490)
==431==    by 0x330598A2: pango_fc_fontset_new (pangofc-fontmap.c:1116)
==431==    by 0x330598A2: pango_fc_font_map_load_fontset (pangofc-fontmap.c:2209)
==431==    by 0x33056C8A: pango_fc_font_map_load_font (pangofc-fontmap.c:2135)
==431==    by 0x2E13E3AD: pango_layout_get_empty_extents_at_index.part.0 (pango-layout.c:4929)
==431==    by 0x2E142265: pango_layout_get_empty_extents_at_index (pango-layout.c:4879)
==431==    by 0x2E142265: pango_layout_check_lines.part.0 (pango-layout.c:4326)
==431==    by 0x2E144368: pango_layout_get_extents_internal (pango-layout.c:2685)
==431==    by 0x2E144802: pango_layout_get_pixel_extents (pango-layout.c:2903)
==431==    by 0x2E0D4B02: gdip_pango_setup_layout (text-pango.c:457)
==431==    by 0x2E0D53DF: pango_MeasureString (text-pango.c:588)
==431==    by 0x5C4B5803: ???
filipnavara commented 2 years ago

I think the custom font map is there to support user-loaded fonts. We would likely need to add locks around the Pango calls.