Bug 141168 - Memory is written to after deallocated, in GraphicsLayer::setMaskLayer.
Summary: Memory is written to after deallocated, in GraphicsLayer::setMaskLayer.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-02 12:35 PST by peavo
Modified: 2015-02-02 13:39 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.56 KB, patch)
2015-02-02 12:40 PST, peavo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.