QBrush: Fix UB (non-virtual dtor) in QBrush::detach()

As the d-pointer, QBrush uses a QScopedPointer with a
custom deleter that checks for QBrushData::style and
casts the QBrushData pointer down to corresponding
subclasses before calling delete on them.

In QBrush::detach(), however, any of the three brush
data classes were held in a QScopedPointer _without_
the custom deleter, invoking UB when that scoped
pointer would ever get to be the one that deleted
the payload instead of handing it over to the objects
d-pointer.

Found by making dtors protected following a Coverity
report wrongly marked as 'dismissed' (these static
checks are not included in this patch, since they
are binary-incompatible), to find out where Coverity
could possibly see a problem.

Also replace the d.reset(x.take()) that allowed this
mismatch to compile with d.swap(x), which nicely
ensures that x and d are of the same type.

Coverity-Id: 11772
Change-Id: I85e2c205df9291bd7508b6c90f7b03fbe8c3bcd2
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Marc Mutz 2016-09-18 16:44:19 +02:00
parent 29d64bc8e0
commit 8082c0dc81

View File

@ -579,7 +579,7 @@ void QBrush::detach(Qt::BrushStyle newStyle)
if (newStyle == d->style && d->ref.load() == 1)
return;
QScopedPointer<QBrushData> x;
QScopedPointer<QBrushData, QBrushDataPointerDeleter> x;
switch(newStyle) {
case Qt::TexturePattern: {
QTexturedBrushData *tbd = new QTexturedBrushData;
@ -595,28 +595,30 @@ void QBrush::detach(Qt::BrushStyle newStyle)
}
case Qt::LinearGradientPattern:
case Qt::RadialGradientPattern:
case Qt::ConicalGradientPattern:
x.reset(new QGradientBrushData);
case Qt::ConicalGradientPattern: {
QGradientBrushData *gbd = new QGradientBrushData;
switch (d->style) {
case Qt::LinearGradientPattern:
case Qt::RadialGradientPattern:
case Qt::ConicalGradientPattern:
static_cast<QGradientBrushData *>(x.data())->gradient =
gbd->gradient =
static_cast<QGradientBrushData *>(d.data())->gradient;
break;
default:
break;
}
x.reset(gbd);
break;
}
default:
x.reset(new QBrushData);
break;
}
x->ref.store(1);
x->ref.store(1); // must be first lest the QBrushDataPointerDeleter turns into a no-op
x->style = newStyle;
x->color = d->color;
x->transform = d->transform;
d.reset(x.take());
d.swap(x);
}