From 8523b6bd0d22083266d990191764a0460885fd6e Mon Sep 17 00:00:00 2001 From: Khushal Date: Tue, 12 Jun 2018 11:26:17 -0700 Subject: 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 Reviewed-by: Mike Klein --- src/core/SkStrikeCache.cpp | 51 +++++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 30 deletions(-) (limited to 'src/core/SkStrikeCache.cpp') 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; } -- cgit v1.2.3