fonts: Fix memory allocation for fallback glyphs.

When allocating the mask for a fallback glyph, we allocate it on the
arena on the SkScalerContext while the image belongs to a glyph on a
different cache. This can lead to use-after-free bugs if accessing the
image after the context owning that memory is destroyed. Fix this by
allocating on the arena from the owning cache.

R=herb@google.com, mtklein@google.com

Bug: 829622
Change-Id: Ife53e24f5bc868f36c43f2adcd7a2629ab5577fe
Reviewed-on: https://skia-review.googlesource.com/134182
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Mike Klein <mtklein@google.com>

1 files changed, 21 insertions, 30 deletions

diff --git a/src/core/SkStrikeCache.cpp b/src/core/SkStrikeCache.cpp
index 20ed6e772c..fef46fd36d 100644
--- a/src/core/SkStrikeCache.cpp
+++ b/src/core/SkStrikeCache.cpp
@@ -101,9 +101,9 @@ SkExclusiveStrikePtr SkStrikeCache::FindStrikeExclusive(const SkDescriptor& desc
     return get_globals().findStrikeExclusive(desc);
 }
 
-bool SkStrikeCache::DesperationSearchForImage(
-        const SkDescriptor& desc, SkGlyph* glyph, SkArenaAlloc* arena) {
-    return get_globals().desperationSearchForImage(desc, glyph, arena);
+bool SkStrikeCache::DesperationSearchForImage(const SkDescriptor& desc, SkGlyph* glyph,
+                                              SkGlyphCache* targetCache) {
+    return get_globals().desperationSearchForImage(desc, glyph, targetCache);
 }
 
 bool SkStrikeCache::DesperationSearchForPath(
@@ -265,7 +265,7 @@ SkExclusiveStrikePtr SkStrikeCache::findStrikeExclusive(const SkDescriptor& desc
     return SkExclusiveStrikePtr(nullptr);
 }
 
-static bool loose_compare (const SkDescriptor& lhs, const SkDescriptor& rhs) {
+static bool loose_compare(const SkDescriptor& lhs, const SkDescriptor& rhs) {
     uint32_t size;
     auto ptr = lhs.findEntry(kRec_SkDescriptorTag, &size);
     SkScalerContextRec lhsRec;
@@ -276,7 +276,10 @@ static bool loose_compare (const SkDescriptor& lhs, const SkDescriptor& rhs) {
     std::memcpy(&rhsRec, ptr, size);
 
     // If these don't match, there's no way we can use these strikes interchangeably.
-    // TODO: make sure we don't search other renderer's caches.
+    // Note that a typeface from each renderer maps to a unique proxy typeface on the GPU,
+    // keyed in the glyph cache using fontID in the SkDescriptor. By limiting this search
+    // to descriptors with the same fontID, we ensure that a renderer never uses glyphs
+    // generated by a different renderer.
     return
         lhsRec.fFontID == rhsRec.fFontID &&
         lhsRec.fTextSize == rhsRec.fTextSize &&
@@ -288,46 +291,34 @@ static bool loose_compare (const SkDescriptor& lhs, const SkDescriptor& rhs) {
         lhsRec.fPost2x2[1][1] == rhsRec.fPost2x2[1][1];
 }
 
-bool SkStrikeCache::desperationSearchForImage(
-        const SkDescriptor& desc, SkGlyph* glyph, SkArenaAlloc* alloc) {
+bool SkStrikeCache::desperationSearchForImage(const SkDescriptor& desc, SkGlyph* glyph,
+                                              SkGlyphCache* targetCache) {
     SkAutoExclusive ac(fLock);
 
     SkGlyphID glyphID = glyph->getGlyphID();
     SkFixed targetSubX = glyph->getSubXFixed(),
             targetSubY = glyph->getSubYFixed();
 
-    // We don't have this glyph with the exact subpixel positioning,
-    // but we might have this glyph with another subpixel position... search them all.
     for (Node* node = internalGetHead(); node != nullptr; node = node->fNext) {
         if (loose_compare(node->fCache.getDescriptor(), desc)) {
             auto targetGlyphID = SkPackedGlyphID(glyphID, targetSubX, targetSubY);
             if (node->fCache.isGlyphCached(glyphID, targetSubX, targetSubY)) {
-                SkGlyph* from = node->fCache.getRawGlyphByID(targetGlyphID);
-                if (from->fImage != nullptr) {
-                    // This desperate-match node may disappear as soon as we drop fLock, so we
-                    // need to copy the glyph from node into this strike, including a
-                    // deep copy of the mask.
-                    glyph->copyImageData(*from, alloc);
-                    return true;
-                }
+                SkGlyph* fallback = node->fCache.getRawGlyphByID(targetGlyphID);
+                // This desperate-match node may disappear as soon as we drop fLock, so we
+                // need to copy the glyph from node into this strike, including a
+                // deep copy of the mask.
+                targetCache->initializeGlyphFromFallback(glyph, *fallback);
+                return true;
             }
 
-            // We don't have this glyph with the exact subpixel positioning,
-            // but we might have this glyph with another subpixel position... search them all.
-            for (SkFixed subY = 0; subY < SK_Fixed1; subY += SK_FixedQuarter) {
-                for (SkFixed subX = 0; subX < SK_Fixed1; subX += SK_FixedQuarter) {
-                    if (node->fCache.isGlyphCached(glyphID, subX, subY)) {
-                        SkGlyph* from =
-                                node->fCache.getRawGlyphByID(SkPackedGlyphID(glyphID, subX, subY));
-                        if (from->fImage != nullptr) {
-                            glyph->copyImageData(*from, alloc);
-                            return true;
-                        }
-                    }
-                }
+            // Look for any sub-pixel pos for this glyph, in case there is a pos mismatch.
+            if (const auto* fallback = node->fCache.getCachedGlyphAnySubPix(glyphID)) {
+                targetCache->initializeGlyphFromFallback(glyph, *fallback);
+                return true;
             }
         }
     }
+
     return false;
 }