- Revision
- 282532
- Author
- [email protected]
- Date
- 2021-09-16 03:36:22 -0700 (Thu, 16 Sep 2021)
Log Message
Merge r278161 - [GPU Process] Add validation when reading DisplayListSharedMemoryHeader::waitingStatus in DisplayListReaderHandle
https://bugs.webkit.org/show_bug.cgi?id=226306
rdar://78118900
Reviewed by Chris Dumez.
Store this waiting status (an enum class with three valid states) as a raw `uint8_t` in shared memory, rather
than an `enum class`. This allows us to verify that this value indicates a valid waiting status (using
`isValidEnum`) upon reading this flag in the GPU process, and perform a `MESSAGE_CHECK` in the case where it is
invalid.
* GPUProcess/graphics/DisplayListReaderHandle.h:
(WebKit::DisplayListReaderHandle::startWaiting):
(WebKit::DisplayListReaderHandle::stopWaiting):
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
* Shared/SessionState.cpp:
(WebKit::PageState::decode):
* Shared/SessionState.h:
Drive-by fix: remove a non-templated declaration of `isValidEnum()`. This is necessary in order to use
`WTF::isValidEnum` without the explicit WTF namespace prefix inside `DisplayListReaderHandle`, since WTF's
declaration of `isValidEnum` clashes with this declaration.
Instead of using this separate method for validating the enum, simply encode and decode the
`shouldOpenExternalURLsPolicy` enum using << and >> operators, respectively; this ensures that the decoded value
of `shouldOpenExternalURLsPolicy` is either valid, or the result will be WTF::nullopt if decoding fails or
results in an invalid enum.
* Shared/SharedDisplayListHandle.h:
* WebProcess/GPU/graphics/DisplayListWriterHandle.h:
(WebKit::DisplayListWriterHandle::tryToResume):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog 2021-09-16 10:36:22 UTC (rev 282532)
@@ -1,3 +1,38 @@
+2021-05-27 Wenson Hsieh <[email protected]>
+
+ [GPU Process] Add validation when reading DisplayListSharedMemoryHeader::waitingStatus in DisplayListReaderHandle
+ https://bugs.webkit.org/show_bug.cgi?id=226306
+ rdar://78118900
+
+ Reviewed by Chris Dumez.
+
+ Store this waiting status (an enum class with three valid states) as a raw `uint8_t` in shared memory, rather
+ than an `enum class`. This allows us to verify that this value indicates a valid waiting status (using
+ `isValidEnum`) upon reading this flag in the GPU process, and perform a `MESSAGE_CHECK` in the case where it is
+ invalid.
+
+ * GPUProcess/graphics/DisplayListReaderHandle.h:
+ (WebKit::DisplayListReaderHandle::startWaiting):
+ (WebKit::DisplayListReaderHandle::stopWaiting):
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
+ * Shared/SessionState.cpp:
+ (WebKit::PageState::decode):
+ * Shared/SessionState.h:
+
+ Drive-by fix: remove a non-templated declaration of `isValidEnum()`. This is necessary in order to use
+ `WTF::isValidEnum` without the explicit WTF namespace prefix inside `DisplayListReaderHandle`, since WTF's
+ declaration of `isValidEnum` clashes with this declaration.
+
+ Instead of using this separate method for validating the enum, simply encode and decode the
+ `shouldOpenExternalURLsPolicy` enum using << and >> operators, respectively; this ensures that the decoded value
+ of `shouldOpenExternalURLsPolicy` is either valid, or the result will be WTF::nullopt if decoding fails or
+ results in an invalid enum.
+
+ * Shared/SharedDisplayListHandle.h:
+ * WebProcess/GPU/graphics/DisplayListWriterHandle.h:
+ (WebKit::DisplayListWriterHandle::tryToResume):
+
2021-05-17 Wenson Hsieh <[email protected]>
[GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h 2021-09-16 10:36:22 UTC (rev 282532)
@@ -26,6 +26,7 @@
#pragma once
#include "SharedDisplayListHandle.h"
+#include <wtf/Expected.h>
#include <wtf/FastMalloc.h>
#include <wtf/Optional.h>
@@ -46,16 +47,21 @@
void startWaiting()
{
- header().waitingStatus.store(SharedDisplayListHandle::WaitingStatus::Waiting);
+ header().waitingStatus.store(static_cast<SharedDisplayListHandle::WaitingStatusStorageType>(SharedDisplayListHandle::WaitingStatus::Waiting));
}
- Optional<SharedDisplayListHandle::ResumeReadingInformation> stopWaiting()
+ enum class StopWaitingFailureReason : uint8_t { InvalidWaitingStatus };
+ Expected<Optional<SharedDisplayListHandle::ResumeReadingInformation>, StopWaitingFailureReason> stopWaiting()
{
auto& header = this->header();
- if (header.waitingStatus.exchange(SharedDisplayListHandle::WaitingStatus::NotWaiting) == SharedDisplayListHandle::WaitingStatus::Resuming)
+ auto previousStatus = header.waitingStatus.exchange(static_cast<SharedDisplayListHandle::WaitingStatusStorageType>(SharedDisplayListHandle::WaitingStatus::NotWaiting));
+ if (!isValidEnum<SharedDisplayListHandle::WaitingStatus>(previousStatus))
+ return makeUnexpected(StopWaitingFailureReason::InvalidWaitingStatus);
+
+ if (static_cast<SharedDisplayListHandle::WaitingStatus>(previousStatus) == SharedDisplayListHandle::WaitingStatus::Resuming)
return { header.resumeReadingInfo };
- return WTF::nullopt;
+ return { WTF::nullopt };
}
private:
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-09-16 10:36:22 UTC (rev 282532)
@@ -244,7 +244,10 @@
sleep(30_us);
#endif
- auto resumeReadingInfo = handle.stopWaiting();
+ auto stopWaitingResult = handle.stopWaiting();
+ MESSAGE_CHECK_WITH_RETURN_VALUE(stopWaitingResult, nullptr, "Invalid waiting status detected when resuming display list processing");
+
+ auto resumeReadingInfo = stopWaitingResult.value();
if (!resumeReadingInfo)
break;
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.cpp (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.cpp 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.cpp 2021-09-16 10:36:22 UTC (rev 282532)
@@ -32,17 +32,6 @@
namespace WebKit {
using namespace WebCore;
-bool isValidEnum(WebCore::ShouldOpenExternalURLsPolicy policy)
-{
- switch (policy) {
- case WebCore::ShouldOpenExternalURLsPolicy::ShouldAllow:
- case WebCore::ShouldOpenExternalURLsPolicy::ShouldAllowExternalSchemesButNotAppLinks:
- case WebCore::ShouldOpenExternalURLsPolicy::ShouldNotAllow:
- return true;
- }
- return false;
-}
-
void HTTPBody::Element::encode(IPC::Encoder& encoder) const
{
encoder << type;
@@ -219,9 +208,12 @@
result.sessionStateObject = SerializedScriptValue::createFromWireBytes(WTFMove(wireBytes));
}
- if (!decoder.decode(result.shouldOpenExternalURLsPolicy) || !isValidEnum(result.shouldOpenExternalURLsPolicy))
+ Optional<ShouldOpenExternalURLsPolicy> shouldOpenExternalURLsPolicy;
+ decoder >> shouldOpenExternalURLsPolicy;
+ if (!shouldOpenExternalURLsPolicy)
return false;
+ result.shouldOpenExternalURLsPolicy = *shouldOpenExternalURLsPolicy;
return true;
}
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.h (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.h 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SessionState.h 2021-09-16 10:36:22 UTC (rev 282532)
@@ -44,8 +44,6 @@
namespace WebKit {
-bool isValidEnum(WebCore::ShouldOpenExternalURLsPolicy);
-
struct HTTPBody {
struct Element {
void encode(IPC::Encoder&) const;
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SharedDisplayListHandle.h (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SharedDisplayListHandle.h 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/Shared/SharedDisplayListHandle.h 2021-09-16 10:36:22 UTC (rev 282532)
@@ -28,6 +28,7 @@
#include "SharedMemory.h"
#include <WebCore/DisplayList.h>
#include <wtf/Atomics.h>
+#include <wtf/EnumTraits.h>
#include <wtf/RefCounted.h>
#include <wtf/Threading.h>
@@ -59,6 +60,7 @@
Waiting,
Resuming
};
+ using WaitingStatusStorageType = std::underlying_type_t<WaitingStatus>;
struct ResumeReadingInformation {
uint64_t offset;
@@ -78,7 +80,7 @@
Atomic<uint64_t> unreadBytes;
ResumeReadingInformation resumeReadingInfo;
- Atomic<WaitingStatus> waitingStatus;
+ Atomic<WaitingStatusStorageType> waitingStatus;
};
const DisplayListSharedMemoryHeader& header() const { return *reinterpret_cast<const DisplayListSharedMemoryHeader*>(data()); }
@@ -90,3 +92,16 @@
};
} // namespace WebKit
+
+namespace WTF {
+
+template<> struct EnumTraits<WebKit::SharedDisplayListHandle::WaitingStatus> {
+ using values = EnumValues<
+ WebKit::SharedDisplayListHandle::WaitingStatus,
+ WebKit::SharedDisplayListHandle::WaitingStatus::NotWaiting,
+ WebKit::SharedDisplayListHandle::WaitingStatus::Waiting,
+ WebKit::SharedDisplayListHandle::WaitingStatus::Resuming
+ >;
+};
+
+} // namespace WTF
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h (282531 => 282532)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h 2021-09-16 10:36:13 UTC (rev 282531)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h 2021-09-16 10:36:22 UTC (rev 282532)
@@ -52,7 +52,10 @@
{
auto& header = this->header();
header.resumeReadingInfo = WTFMove(info);
- return header.waitingStatus.compareExchangeWeak(SharedDisplayListHandle::WaitingStatus::Waiting, SharedDisplayListHandle::WaitingStatus::Resuming);
+ return header.waitingStatus.compareExchangeWeak(
+ static_cast<SharedDisplayListHandle::WaitingStatusStorageType>(SharedDisplayListHandle::WaitingStatus::Waiting),
+ static_cast<SharedDisplayListHandle::WaitingStatusStorageType>(SharedDisplayListHandle::WaitingStatus::Resuming)
+ );
}
private:
