From f98c2a67fe051cc92f3205bfc0267ddac90d3a71 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Mon, 5 Dec 2011 20:37:44 -0800 Subject: [PATCH] Fix memory corruption issues in TextLayoutCache. (DO NOT MERGE) Ensure log_clusters array is big enough. Bug: 5714171 Explicitly handle the cases where the entire string or a single run might have a length of 0. Harfbuzz assumes the length of the item is at least 1. If the length is zero, then it will clobber memory at index -1 into the log_clusters array. Bug: 5705479 Change-Id: If28a9866221081f69973c1d12d7fe0cf8db2edd0 --- core/jni/android/graphics/TextLayoutCache.cpp | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/core/jni/android/graphics/TextLayoutCache.cpp b/core/jni/android/graphics/TextLayoutCache.cpp index 7076e2a84eb31..510e541b18495 100644 --- a/core/jni/android/graphics/TextLayoutCache.cpp +++ b/core/jni/android/graphics/TextLayoutCache.cpp @@ -368,9 +368,6 @@ void TextLayoutCacheValue::initShaperItem(HB_ShaperItem& shaperItem, HB_FontRec* // padding and fallback if we find that we are wrong. createGlyphArrays(shaperItem, (contextCount + 2) * 2); - // Create log clusters array - shaperItem.log_clusters = new unsigned short[contextCount]; - // Set the string properties shaperItem.string = chars; shaperItem.stringLength = contextCount; @@ -378,7 +375,6 @@ void TextLayoutCacheValue::initShaperItem(HB_ShaperItem& shaperItem, HB_FontRec* void TextLayoutCacheValue::freeShaperItem(HB_ShaperItem& shaperItem) { deleteGlyphArrays(shaperItem); - delete[] shaperItem.log_clusters; HB_FreeFace(shaperItem.face); } @@ -392,6 +388,7 @@ void TextLayoutCacheValue::shapeRun(HB_ShaperItem& shaperItem, size_t start, siz shaperItem.item.script = isRTL ? HB_Script_Arabic : HB_Script_Common; // Shape + assert(shaperItem.item.length > 0); // Harfbuzz will overwrite other memory if length is 0. while (!HB_ShapeItem(&shaperItem)) { // We overflowed our arrays. Resize and retry. // HB_ShapeItem fills in shaperItem.num_glyphs with the needed size. @@ -404,6 +401,10 @@ void TextLayoutCacheValue::computeValuesWithHarfbuzz(SkPaint* paint, const UChar size_t start, size_t count, size_t contextCount, int dirFlags, Vector* const outAdvances, jfloat* outTotalAdvance, Vector* const outGlyphs) { + if (!count) { + *outTotalAdvance = 0; + return; + } UBiDiLevel bidiReq = 0; bool forceLTR = false; @@ -544,6 +545,10 @@ void TextLayoutCacheValue::computeRunValuesWithHarfbuzz(HB_ShaperItem& shaperIte size_t start, size_t count, bool isRTL, Vector* const outAdvances, jfloat* outTotalAdvance, Vector* const outGlyphs) { + if (!count) { + *outTotalAdvance = 0; + return; + } shapeRun(shaperItem, start, count, isRTL); @@ -607,14 +612,22 @@ void TextLayoutCacheValue::deleteGlyphArrays(HB_ShaperItem& shaperItem) { delete[] shaperItem.attributes; delete[] shaperItem.advances; delete[] shaperItem.offsets; + delete[] shaperItem.log_clusters; } void TextLayoutCacheValue::createGlyphArrays(HB_ShaperItem& shaperItem, int size) { + shaperItem.num_glyphs = size; + + // These arrays are all indexed by glyph shaperItem.glyphs = new HB_Glyph[size]; shaperItem.attributes = new HB_GlyphAttributes[size]; shaperItem.advances = new HB_Fixed[size]; shaperItem.offsets = new HB_FixedPoint[size]; - shaperItem.num_glyphs = size; + + // Although the log_clusters array is indexed by character, Harfbuzz expects that + // it is big enough to hold one element per glyph. So we allocate log_clusters along + // with the other glyph arrays above. + shaperItem.log_clusters = new unsigned short[size]; } } // namespace android