Bug 141168 – Memory is written to after deallocated, in GraphicsLayer::setMaskLayer.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

Visual Studio detected that a deallocated heap block had been modified in GraphicsLayer::setMaskLayer, when called from RenderLayerBacking::updateChildClippingStrategy.

void GraphicsLayer::setMaskLayer(GraphicsLayer* layer)
{
    if (layer == m_maskLayer)
        return;

    if (layer) {
        layer->removeFromParent();
        layer->setParent(this);
        layer->setIsMaskLayer(true);
    } else if (m_maskLayer) {
        m_maskLayer->setParent(nullptr);    <--------------- Writing to deallocated memory
        m_maskLayer->setIsMaskLayer(false); <---------------
    }
    
    m_maskLayer = layer;
}

Created attachment 245894 [details]
Patch

[Brent Fulgham]

Comment on attachment 245894 [details]
Patch

Wow! That's not good! :-)

I guess this happens if the m_childClippingMaskLayer is also part of the layer hierarchy and is therefore accessed for a "setMaskLayer" update?

This might only happen in the WinCairo implementation due to its use of the texture mapping stuff to handle accelerated compositing.

r=me.

(In reply to comment #2)
> Comment on attachment 245894 [details]
> Patch
> 

Thanks for reviewing :)

> Wow! That's not good! :-)
> 
> I guess this happens if the m_childClippingMaskLayer is also part of the
> layer hierarchy and is therefore accessed for a "setMaskLayer" update?
> 
> This might only happen in the WinCairo implementation due to its use of the
> texture mapping stuff to handle accelerated compositing.
> 

Good point, could be a bug only on WinCairo.

Also, it was not really harmful, since the overwrite happened just after deallocation, and nobody had reallocated the block, yet ... :)

> r=me.

[WebKit Commit Bot]

Comment on attachment 245894 [details]
Patch

Clearing flags on attachment: 245894

Committed r179495: <http://trac.webkit.org/changeset/179495>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.