Sharing SharedBuffer between WebCore and ImageIO is racy and crash prone
authorpsolanki@apple.com <psolanki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jul 2014 21:37:43 +0000 (21:37 +0000)
committerpsolanki@apple.com <psolanki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Jul 2014 21:37:43 +0000 (21:37 +0000)
commitce1916c2e3566b3f29b1375603097ce49e02c8ff
tree4c80efd628c7d03ae9301d0759aeb762cd2bd21f
parent091155f3be2a210d93479c015cad3bc2cda01731
Sharing SharedBuffer between WebCore and ImageIO is racy and crash prone
https://bugs.webkit.org/show_bug.cgi?id=135069
<rdar://problem/17470655>

Reviewed by Simon Fraser.

When passing image data to ImageIO for decoding, we pass an NSData subclass that is a wraper
around SharedBuffer. This can be a problem when ImageIO tries to access the data on the CA
thread. End result is data corruption on large image loads and potential crashes. The fix is
to have SharedBuffer create a copy of its data if the data has been passed to ImageIO and
might be accessed concurrently.

Since Vector is not refcounted, we do this by having a new refcounted object in SharedBuffer
that contains the buffer and we pass that in our NSData subclass WebCoreSharedBufferData.
Code that would result in the Vector memory moving e.g. append(), resize(), now checks to
see if the buffer was shared and if so, will create a new copy of the vector. This ensures
that the main thread does not end up invalidating the vector memory that we have passed it
to ImageIO.

No new tests because no functional changes.

* loader/cache/CachedResource.cpp:
(WebCore::CachedResource::makePurgeable):
    Remove early return - createPurgeableMemory() has the correct check now.
* platform/SharedBuffer.cpp:
(WebCore::SharedBuffer::SharedBuffer):
(WebCore::SharedBuffer::adoptVector):
(WebCore::SharedBuffer::createPurgeableBuffer):
    Don't create purgeable buffer if we are sharing the buffer.
(WebCore::SharedBuffer::append):
(WebCore::SharedBuffer::clear):
(WebCore::SharedBuffer::copy):
(WebCore::SharedBuffer::duplicateDataBufferIfNecessary): Added.
    Create a new copy of the data if we have shared the buffer and if appending to it would
    exceed the capacity of the vector resulting in memmove.
(WebCore::SharedBuffer::appendToInternalBuffer): Added.
(WebCore::SharedBuffer::clearInternalBuffer): Added.
(WebCore::SharedBuffer::buffer):
    Create a new copy of the buffer if we have shared it.
(WebCore::SharedBuffer::getSomeData):
* platform/SharedBuffer.h:
* platform/cf/SharedBufferCF.cpp:
(WebCore::SharedBuffer::SharedBuffer):
(WebCore::SharedBuffer::singleDataArrayBuffer):
(WebCore::SharedBuffer::maybeAppendDataArray):
* platform/mac/SharedBufferMac.mm:
    Pass the InternalBuffer object to WebCoreSharedBufferData
(-[WebCoreSharedBufferData dealloc]):
(-[WebCoreSharedBufferData initWithSharedBufferInternalBuffer:]):
(-[WebCoreSharedBufferData length]):
(-[WebCoreSharedBufferData bytes]):
(WebCore::SharedBuffer::createNSData):
    Call createCFData() instead of duplicating code.
(WebCore::SharedBuffer::createCFData):
    If the data is in purgeable memory, make a copy of it since m_buffer was cleared when
    creating the purgeable buffer.
(-[WebCoreSharedBufferData initWithSharedBuffer:]): Deleted.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@171526 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Source/WebCore/ChangeLog
Source/WebCore/loader/cache/CachedResource.cpp
Source/WebCore/platform/SharedBuffer.cpp
Source/WebCore/platform/SharedBuffer.h
Source/WebCore/platform/cf/SharedBufferCF.cpp
Source/WebCore/platform/mac/SharedBufferMac.mm