From 9ad4157530e86a1bac7ab8ca50ba3ee9f839f536 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 10 Oct 2016 11:30:51 +0200 Subject: [PATCH] Fix gradient race condition / read-after-free A gradient table may be deallocated while in use because we don't keep track of references. To fix it we now reference count the cache entries. Task-number: QTBUG-14614 Change-Id: I772ebf565ccf41d476811ca9a51b721f10de8aeb Reviewed-by: Marc Mutz --- src/gui/painting/qdrawhelper_p.h | 2 + src/gui/painting/qpaintengine_raster.cpp | 51 ++++++++++++------------ 2 files changed, 27 insertions(+), 26 deletions(-) diff --git a/src/gui/painting/qdrawhelper_p.h b/src/gui/painting/qdrawhelper_p.h index d636eabe3f..1c6cd5db8a 100644 --- a/src/gui/painting/qdrawhelper_p.h +++ b/src/gui/painting/qdrawhelper_p.h @@ -329,6 +329,8 @@ struct QSpanData QGradientData gradient; QTextureData texture; }; + QExplicitlySharedDataPointer cachedGradient; + void init(QRasterBuffer *rb, const QRasterPaintEngine *pe); void setup(const QBrush &brush, int alpha, QPainter::CompositionMode compositionMode); diff --git a/src/gui/painting/qpaintengine_raster.cpp b/src/gui/painting/qpaintengine_raster.cpp index 278d7bb99e..f87b052df2 100644 --- a/src/gui/painting/qpaintengine_raster.cpp +++ b/src/gui/painting/qpaintengine_raster.cpp @@ -4137,7 +4137,8 @@ void QRasterBuffer::flushToARGBImage(QImage *target) const class QGradientCache { - struct CacheInfo +public: + struct CacheInfo : public QSharedData { inline CacheInfo(QGradientStops s, int op, QGradient::InterpolationMode mode) : stops(qMove(s)), opacity(op), interpolationMode(mode) {} @@ -4148,12 +4149,9 @@ class QGradientCache QGradient::InterpolationMode interpolationMode; }; - typedef QMultiHash QGradientColorTableHash; + typedef QMultiHash > QGradientColorTableHash; -public: - typedef QPair ColorBufferPair; - - inline ColorBufferPair getBuffer(const QGradient &gradient, int opacity) { + inline QExplicitlySharedDataPointer getBuffer(const QGradient &gradient, int opacity) { quint64 hash_val = 0; const QGradientStops stops = gradient.stops(); @@ -4167,10 +4165,9 @@ public: return addCacheElement(hash_val, gradient, opacity); else { do { - const CacheInfo &cache_info = it.value(); - if (cache_info.stops == stops && cache_info.opacity == opacity && cache_info.interpolationMode == gradient.interpolationMode()) - return qMakePair(reinterpret_cast(cache_info.buffer32), - reinterpret_cast(cache_info.buffer64)); + const QExplicitlySharedDataPointer &cache_info = it.value(); + if (cache_info->stops == stops && cache_info->opacity == opacity && cache_info->interpolationMode == gradient.interpolationMode()) + return cache_info; ++it; } while (it != cache.constEnd() && it.key() == hash_val); // an exact match for these stops and opacity was not found, create new cache @@ -4184,18 +4181,16 @@ protected: inline void generateGradientColorTable(const QGradient& g, QRgba64 *colorTable, int size, int opacity) const; - ColorBufferPair addCacheElement(quint64 hash_val, const QGradient &gradient, int opacity) { + QExplicitlySharedDataPointer addCacheElement(quint64 hash_val, const QGradient &gradient, int opacity) { if (cache.size() == maxCacheSize()) { // may remove more than 1, but OK cache.erase(cache.begin() + (qrand() % maxCacheSize())); } - CacheInfo cache_entry(gradient.stops(), opacity, gradient.interpolationMode()); - generateGradientColorTable(gradient, cache_entry.buffer64, paletteSize(), opacity); + QExplicitlySharedDataPointer cache_entry(new CacheInfo (gradient.stops(), opacity, gradient.interpolationMode())); + generateGradientColorTable(gradient, cache_entry->buffer64, paletteSize(), opacity); for (int i = 0; i < GRADIENT_STOPTABLE_SIZE; ++i) - cache_entry.buffer32[i] = cache_entry.buffer64[i].toArgb32(); - CacheInfo &cache_value = cache.insert(hash_val, cache_entry).value(); - return qMakePair(reinterpret_cast(cache_value.buffer32), - reinterpret_cast(cache_value.buffer64)); + cache_entry->buffer32[i] = cache_entry->buffer64[i].toArgb32(); + return cache.insert(hash_val, cache_entry).value(); } QGradientColorTableHash cache; @@ -4414,6 +4409,7 @@ Q_GUI_EXPORT extern QImage qt_imageForBrush(int brushStyle, bool invert); void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode compositionMode) { Qt::BrushStyle brushStyle = qbrush_style(brush); + cachedGradient.reset(); switch (brushStyle) { case Qt::SolidPattern: { type = Solid; @@ -4430,9 +4426,10 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode const QLinearGradient *g = static_cast(brush.gradient()); gradient.alphaColor = !brush.isOpaque() || alpha != 256; - QGradientCache::ColorBufferPair colorBuffers = qt_gradient_cache()->getBuffer(*g, alpha); - gradient.colorTable64 = colorBuffers.second; - gradient.colorTable32 = colorBuffers.first; + QExplicitlySharedDataPointer cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha); + cachedGradient = cacheInfo; + gradient.colorTable32 = cacheInfo->buffer32; + gradient.colorTable64 = cacheInfo->buffer64; gradient.spread = g->spread(); @@ -4451,9 +4448,10 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode const QRadialGradient *g = static_cast(brush.gradient()); gradient.alphaColor = !brush.isOpaque() || alpha != 256; - QGradientCache::ColorBufferPair colorBuffers = qt_gradient_cache()->getBuffer(*g, alpha); - gradient.colorTable64 = colorBuffers.second; - gradient.colorTable32 = colorBuffers.first; + QExplicitlySharedDataPointer cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha); + cachedGradient = cacheInfo; + gradient.colorTable32 = cacheInfo->buffer32; + gradient.colorTable64 = cacheInfo->buffer64; gradient.spread = g->spread(); @@ -4476,9 +4474,10 @@ void QSpanData::setup(const QBrush &brush, int alpha, QPainter::CompositionMode const QConicalGradient *g = static_cast(brush.gradient()); gradient.alphaColor = !brush.isOpaque() || alpha != 256; - QGradientCache::ColorBufferPair colorBuffers = qt_gradient_cache()->getBuffer(*g, alpha); - gradient.colorTable64 = colorBuffers.second; - gradient.colorTable32 = colorBuffers.first; + QExplicitlySharedDataPointer cacheInfo = qt_gradient_cache()->getBuffer(*g, alpha); + cachedGradient = cacheInfo; + gradient.colorTable32 = cacheInfo->buffer32; + gradient.colorTable64 = cacheInfo->buffer64; gradient.spread = QGradient::RepeatSpread;