Bug 704212: Fix race condition in fz_advance_glyph.

Suppose one thread calls fz_advance_glyph, determines that we should
be using an advance cache, but one doesn't exist. It will then
allocate it, and start to fill it.

If at that point another thread calls fz_advance_glyph, it'll see
the newly created advance cache, and access it regardless of the
fact that it's newly only partially filled.

The way to avoid this is to hold the FREETYPE lock around the
use of the advance_cache.

diff --git a/source/fitz/font.c b/source/fitz/font.c
index 67196fadc5f216a86a07bbb7d638ba1bab0a1ba0..43e5db59f703339c6a5433bb118195020e55e3bb 100644 (file)
--- a/source/fitz/font.c
+++ b/source/fitz/font.c
@@ -1701,7 +1701,7 @@ int fz_glyph_cacheable(fz_context *ctx, fz_font *font, int gid)
 }
 
 static float
-fz_advance_ft_glyph(fz_context *ctx, fz_font *font, int gid, int wmode)
+fz_advance_ft_glyph_aux(fz_context *ctx, fz_font *font, int gid, int wmode, int locked)
 {
        FT_Error fterr;
        FT_Fixed adv = 0;
@@ -1721,9 +1721,11 @@ fz_advance_ft_glyph(fz_context *ctx, fz_font *font, int gid, int wmode)
        mask = FT_LOAD_NO_SCALE | FT_LOAD_NO_HINTING | FT_LOAD_IGNORE_TRANSFORM;
        if (wmode)
                mask |= FT_LOAD_VERTICAL_LAYOUT;
-       fz_lock(ctx, FZ_LOCK_FREETYPE);
+       if (!locked)
+               fz_lock(ctx, FZ_LOCK_FREETYPE);
        fterr = FT_Get_Advance(font->ft_face, gid, mask, &adv);
-       fz_unlock(ctx, FZ_LOCK_FREETYPE);
+       if (!locked)
+               fz_unlock(ctx, FZ_LOCK_FREETYPE);
        if (fterr && fterr != FT_Err_Invalid_Argument)
        {
                fz_warn(ctx, "FT_Get_Advance(%s,%d): %s", font->name, gid, ft_error_string(fterr));
@@ -1737,6 +1739,12 @@ fz_advance_ft_glyph(fz_context *ctx, fz_font *font, int gid, int wmode)
        return (float) adv / ((FT_Face)font->ft_face)->units_per_EM;
 }
 
+static float
+fz_advance_ft_glyph(fz_context *ctx, fz_font *font, int gid, int wmode)
+{
+       return fz_advance_ft_glyph_aux(ctx, font, gid, wmode, 0);
+}
+
 static float
 fz_advance_t3_glyph(fz_context *ctx, fz_font *font, int gid)
 {
@@ -1775,14 +1783,24 @@ fz_advance_glyph(fz_context *ctx, fz_font *font, int gid, int wmode)
                        return fz_advance_ft_glyph(ctx, font, gid, 1);
                if (gid >= 0 && gid < font->glyph_count && gid < MAX_ADVANCE_CACHE)
                {
+                       float f;
+                       fz_lock(ctx, FZ_LOCK_FREETYPE);
                        if (!font->advance_cache)
                        {
                                int i;
-                               font->advance_cache = Memento_label(fz_malloc_array(ctx, font->glyph_count, float), "font_advance_cache");
+                               fz_try(ctx)
+                                       font->advance_cache = Memento_label(fz_malloc_array(ctx, font->glyph_count, float), "font_advance_cache");
+                               fz_catch(ctx)
+                               {
+                                       fz_unlock(ctx, FZ_LOCK_FREETYPE);
+                                       fz_rethrow(ctx);
+                               }
                                for (i = 0; i < font->glyph_count; ++i)
-                                       font->advance_cache[i] = fz_advance_ft_glyph(ctx, font, i, 0);
+                                       font->advance_cache[i] = fz_advance_ft_glyph_aux(ctx, font, i, 0, 1);
                        }
-                       return font->advance_cache[gid];
+                       f = font->advance_cache[gid];
+                       fz_unlock(ctx, FZ_LOCK_FREETYPE);
+                       return f;
                }
 
                return fz_advance_ft_glyph(ctx, font, gid, 0);