Bug 141168

Summary: Memory is written to after deallocated, in GraphicsLayer::setMaskLayer.
Product: WebKit Reporter: peavo
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: alex.christensen, bfulgham, commit-queue, esprehn+autocc, glenn, kondapallykalyan, simon.fraser
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description peavo 2015-02-02 12:35:34 PST
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;
}
Comment 1 peavo 2015-02-02 12:40:43 PST
Created attachment 245894 [details]
Patch
Comment 2 Brent Fulgham 2015-02-02 12:56:21 PST
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.
Comment 3 peavo 2015-02-02 13:13:08 PST
(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.
Comment 4 WebKit Commit Bot 2015-02-02 13:39:09 PST
Comment on attachment 245894 [details]
Patch

Clearing flags on attachment: 245894

Committed r179495: <http://trac.webkit.org/changeset/179495>
Comment 5 WebKit Commit Bot 2015-02-02 13:39:13 PST
All reviewed patches have been landed.  Closing bug.