Changeset 271772 in webkit


Ignore:
Timestamp:
Jan 22, 2021 6:13:17 PM (3 years ago)
Author:
Wenson Hsieh
Message:

The web process should be killed after failing to decode display list items in the GPU process
​https://bugs.webkit.org/show_bug.cgi?id=219097
<rdar://problem/71546526>

Reviewed by Chris Dumez.

Handle StopReplayReason::InvalidItem by terminating the web process via MESSAGE_CHECK. See below for more
details.

  • GPUProcess/GPUConnectionToWebProcess.cpp:

(WebKit::GPUConnectionToWebProcess::didReceiveInvalidMessage):
(WebKit::GPUConnectionToWebProcess::terminateWebProcess):

  • GPUProcess/GPUConnectionToWebProcess.h:

Pull logic for terminating the web process out into a separate helper method on GPUConnectionToWebProcess, and
use this helper in GPUConnectionToWebProcess::didReceiveInvalidMessage, as well as in RemoteRenderingBackend
below.

  • GPUProcess/graphics/RemoteRenderingBackend.cpp:

Add macro definitions for MESSAGE_CHECK and MESSAGE_CHECK_WITH_RETURN_VALUE. Since the methods on
RemoteRenderingBackend that trigger message checks all run on a background queue, the normal (main-thread) way
of defining these macros (MESSAGE_CHECK_WITH_RETURN_VALUE_BASE and MESSAGE_CHECK_BASE) don't work. We
instead call into the GPU to web process connection object directly to send a TerminateWebProcess message to
the parent (UI) process, and additionally log a given failure message.

(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):

Replace a number FIXMEs throughout these IPC message handlers with MESSAGE_CHECKs. Additionally, add a new
MESSAGE_CHECK for the case where we stopped replay early due to a corrupted or invalid display list item.

(WebKit::RemoteRenderingBackend::wakeUpAndApplyDisplayList):
(WebKit::RemoteRenderingBackend::setNextItemBufferToRead):

Simply remove the FIXME and early return here; this can only happen in the case where a compromised web content
process attempts to append redundant item buffer change items. However, doing so will either be (1) harmless,
since the pending item buffer information will just be overwritten, or (2) result in hitting a MESSAGE_CHECK
when decoding items if we try to execute the contents of another item buffer that has not been written to.

(WebKit::RemoteRenderingBackend::didCreateSharedDisplayListHandle):

  • GPUProcess/graphics/RemoteRenderingBackend.h:
Location:
trunk/Source/WebKit
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r271771 r271772  
     12021-01-22  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        The web process should be killed after failing to decode display list items in the GPU process
     4        https://bugs.webkit.org/show_bug.cgi?id=219097
     5        <rdar://problem/71546526>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Handle `StopReplayReason::InvalidItem` by terminating the web process via MESSAGE_CHECK. See below for more
     10        details.
     11
     12        * GPUProcess/GPUConnectionToWebProcess.cpp:
     13        (WebKit::GPUConnectionToWebProcess::didReceiveInvalidMessage):
     14        (WebKit::GPUConnectionToWebProcess::terminateWebProcess):
     15        * GPUProcess/GPUConnectionToWebProcess.h:
     16
     17        Pull logic for terminating the web process out into a separate helper method on `GPUConnectionToWebProcess`, and
     18        use this helper in `GPUConnectionToWebProcess::didReceiveInvalidMessage`, as well as in `RemoteRenderingBackend`
     19        below.
     20
     21        * GPUProcess/graphics/RemoteRenderingBackend.cpp:
     22
     23        Add macro definitions for `MESSAGE_CHECK` and `MESSAGE_CHECK_WITH_RETURN_VALUE`. Since the methods on
     24        `RemoteRenderingBackend` that trigger message checks all run on a background queue, the normal (main-thread) way
     25        of defining these macros (`MESSAGE_CHECK_WITH_RETURN_VALUE_BASE` and `MESSAGE_CHECK_BASE`) don't work. We
     26        instead call into the GPU to web process connection object directly to send a `TerminateWebProcess` message to
     27        the parent (UI) process, and additionally log a given failure message.
     28
     29        (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
     30
     31        Replace a number FIXMEs throughout these IPC message handlers with `MESSAGE_CHECK`s. Additionally, add a new
     32        `MESSAGE_CHECK` for the case where we stopped replay early due to a corrupted or invalid display list item.
     33
     34        (WebKit::RemoteRenderingBackend::wakeUpAndApplyDisplayList):
     35        (WebKit::RemoteRenderingBackend::setNextItemBufferToRead):
     36
     37        Simply remove the FIXME and early return here; this can only happen in the case where a compromised web content
     38        process attempts to append redundant item buffer change items. However, doing so will either be (1) harmless,
     39        since the pending item buffer information will just be overwritten, or (2) result in hitting a MESSAGE_CHECK
     40        when decoding items if we try to execute the contents of another item buffer that has not been written to.
     41
     42        (WebKit::RemoteRenderingBackend::didCreateSharedDisplayListHandle):
     43        * GPUProcess/graphics/RemoteRenderingBackend.h:
     44
    1452021-01-22  Simon Fraser  <simon.fraser@apple.com>
    246

  • trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp

    r271533 r271772  
    225225{
    226226    RELEASE_LOG_FAULT(IPC, "Received an invalid message '%" PUBLIC_LOG_STRING "' from WebContent process %" PRIu64 ", requesting for it to be terminated.", description(messageName), m_webProcessIdentifier.toUInt64());
     227    terminateWebProcess();
     228}
     229
     230void GPUConnectionToWebProcess::terminateWebProcess()
     231{
    227232    gpuProcess().parentProcessConnection()->send(Messages::GPUProcessProxy::TerminateWebProcess(m_webProcessIdentifier), 0);
    228233}

  • trunk/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h

    r271533 r271772  
    113113#endif
    114114
     115    void terminateWebProcess();
     116
    115117private:
    116118    GPUConnectionToWebProcess(GPUProcess&, WebCore::ProcessIdentifier, IPC::Connection::Identifier, PAL::SessionID);

  • trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp

    r271533 r271772  
    4545#endif
    4646
     47#define TERMINATE_WEB_PROCESS_WITH_MESSAGE(message) \
     48    RELEASE_LOG_FAULT(IPC, "Requesting termination of web process %" PRIu64 " for reason: %" PUBLIC_LOG_STRING, m_gpuConnectionToWebProcess->webProcessIdentifier().toUInt64(), #message); \
     49    m_gpuConnectionToWebProcess->terminateWebProcess();
     50
     51#define MESSAGE_CHECK(assertion, message) do { \
     52    if (UNLIKELY(!(assertion))) { \
     53        TERMINATE_WEB_PROCESS_WITH_MESSAGE(message); \
     54        return; \
     55    } \
     56} while (0)
     57
     58#define MESSAGE_CHECK_WITH_RETURN_VALUE(assertion, returnValue, message) do { \
     59    if (UNLIKELY(!(assertion))) { \
     60        TERMINATE_WEB_PROCESS_WITH_MESSAGE(message); \
     61        return (returnValue); \
     62    } \
     63} while (0)
     64
    4765namespace WebKit {
    4866using namespace WebCore;
    … …  
    174192    while (destination) {
    175193        auto displayList = handle.displayListForReading(offset, sizeToRead, *this);
    176         if (UNLIKELY(!displayList)) {
    177             // FIXME: Add a message check to terminate the web process.
    178             ASSERT_NOT_REACHED();
    179             break;
    180         }
     194        MESSAGE_CHECK_WITH_RETURN_VALUE(displayList, nullptr, "Failed to map display list from shared memory");
    181195
    182196        auto result = submit(*displayList, *destination);
    183197        sizeToRead = handle.advance(result.numberOfBytesRead);
     198        MESSAGE_CHECK_WITH_RETURN_VALUE(result.reasonForStopping != DisplayList::StopReplayReason::InvalidItem, nullptr, "Detected invalid display list item");
    184199
    185200        CheckedSize checkedOffset = offset;
    186201        checkedOffset += result.numberOfBytesRead;
    187         if (UNLIKELY(checkedOffset.hasOverflowed())) {
    188             // FIXME: Add a message check to terminate the web process.
    189             ASSERT_NOT_REACHED();
    190             break;
    191         }
     202        MESSAGE_CHECK_WITH_RETURN_VALUE(!checkedOffset.hasOverflowed(), nullptr, "Overflowed when advancing shared display list handle offset");
    192203
    193204        offset = checkedOffset.unsafeGet();
    194 
    195         if (UNLIKELY(offset > handle.sharedMemory().size())) {
    196             // FIXME: Add a message check to terminate the web process.
    197             ASSERT_NOT_REACHED();
    198             break;
    199         }
     205        MESSAGE_CHECK_WITH_RETURN_VALUE(offset <= handle.sharedMemory().size(), nullptr, "Out-of-bounds offset into shared display list handle");
    200206
    201207        if (result.reasonForStopping == DisplayList::StopReplayReason::ChangeDestinationImageBuffer) {
    … …  
    233239
    234240            sizeToRead = handle.unreadBytes();
    235             if (UNLIKELY(!sizeToRead)) {
    236                 // FIXME: Add a message check to terminate the web process.
    237                 ASSERT_NOT_REACHED();
    238                 break;
    239             }
     241            MESSAGE_CHECK_WITH_RETURN_VALUE(sizeToRead, nullptr, "No unread bytes when resuming display list processing");
    240242
    241243            auto newDestinationIdentifier = makeObjectIdentifier<RenderingResourceIdentifierType>(resumeReadingInfo->destination);
    242             if (UNLIKELY(!newDestinationIdentifier)) {
    243                 // FIXME: Add a message check to terminate the web process.
    244                 ASSERT_NOT_REACHED();
    245                 break;
    246             }
     244            MESSAGE_CHECK_WITH_RETURN_VALUE(newDestinationIdentifier, nullptr, "Invalid image buffer destination when resuming display list processing");
    247245
    248246            destination = makeRefPtr(m_remoteResourceCache.cachedImageBuffer(newDestinationIdentifier));
    249 
    250             if (UNLIKELY(!destination)) {
    251                 // FIXME: Add a message check to terminate the web process.
    252                 ASSERT_NOT_REACHED();
    253                 break;
    254             }
     247            MESSAGE_CHECK_WITH_RETURN_VALUE(destination, nullptr, "Missing image buffer destination when resuming display list processing");
    255248
    256249            offset = resumeReadingInfo->offset;
    … …  
    273266    TraceScope tracingScope(WakeUpAndApplyDisplayListStart, WakeUpAndApplyDisplayListEnd);
    274267    auto destinationImageBuffer = makeRefPtr(m_remoteResourceCache.cachedImageBuffer(arguments.destinationImageBufferIdentifier));
    275     if (UNLIKELY(!destinationImageBuffer)) {
    276         // FIXME: Add a message check to terminate the web process.
    277         ASSERT_NOT_REACHED();
     268    MESSAGE_CHECK(destinationImageBuffer, "Missing destination image buffer");
     269
     270    auto initialHandle = m_sharedDisplayListHandles.get(arguments.itemBufferIdentifier);
     271    MESSAGE_CHECK(initialHandle, "Missing initial shared display list handle");
     272
     273    destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, arguments.offset, *initialHandle, arguments.reason);
     274    if (!destinationImageBuffer)
    278275        return;
    279     }
    280 
    281     auto initialHandle = m_sharedDisplayListHandles.get(arguments.itemBufferIdentifier);
    282     if (UNLIKELY(!initialHandle)) {
    283         // FIXME: Add a message check to terminate the web process.
    284         ASSERT_NOT_REACHED();
    285         return;
    286     }
    287 
    288     destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, arguments.offset, *initialHandle, arguments.reason);
    289     if (!destinationImageBuffer) {
    290         RELEASE_ASSERT(m_pendingWakeupInfo);
    291         return;
    292     }
    293276
    294277    while (m_pendingWakeupInfo) {
    … …  
    306289        auto arguments = std::exchange(m_pendingWakeupInfo, WTF::nullopt)->arguments;
    307290        destinationImageBuffer = nextDestinationImageBufferAfterApplyingDisplayLists(*destinationImageBuffer, arguments.offset, *nextHandle, arguments.reason);
    308         if (!destinationImageBuffer) {
    309             RELEASE_ASSERT(m_pendingWakeupInfo);
     291        if (!destinationImageBuffer)
    310292            break;
    311         }
    312293    }
    313294}
    … …  
    315296void RemoteRenderingBackend::setNextItemBufferToRead(DisplayList::ItemBufferIdentifier identifier, WebCore::RenderingResourceIdentifier destinationIdentifier)
    316297{
    317     if (UNLIKELY(m_pendingWakeupInfo)) {
    318         // FIXME: Add a message check to terminate the web process.
    319         ASSERT_NOT_REACHED();
    320         return;
    321     }
    322298    m_pendingWakeupInfo = {{{ identifier, SharedDisplayListHandle::headerSize(), destinationIdentifier, GPUProcessWakeupReason::Unspecified }, WTF::nullopt }};
    323299}
    … …  
    406382{
    407383    ASSERT(!RunLoop::isMain());
    408 
    409     if (UNLIKELY(m_sharedDisplayListHandles.contains(identifier))) {
    410         // FIXME: Add a message check to terminate the web process.
    411         ASSERT_NOT_REACHED();
    412         return;
    413     }
     384    MESSAGE_CHECK(!m_sharedDisplayListHandles.contains(identifier), "Duplicate shared display list handle");
    414385
    415386    if (auto sharedMemory = SharedMemory::map(handle.handle, SharedMemory::Protection::ReadWrite))
Note: See TracChangeset for help on using the changeset viewer.