Title: [271800] trunk/Source/WebKit
- Revision
- 271800
- Author
- wenson_hs...@apple.com
- Date
- 2021-01-25 10:33:19 -0800 (Mon, 25 Jan 2021)
Log Message
[GPU Process] Web process should be terminated if DisplayListReaderHandle advances past 0
https://bugs.webkit.org/show_bug.cgi?id=220926
Reviewed by Simon Fraser.
Address another FIXME in the GPU Process by replacing a release assertion with a MESSAGE_CHECK. See below for
more details.
* GPUProcess/graphics/DisplayListReaderHandle.cpp:
(WebKit::DisplayListReaderHandle::advance):
Return `WTF::nullopt` instead of just crashing in the case where the number of bytes to advance exceeds the
number of unread bytes. This can (and should) only happen in the case where a compromised web content process
attempts to overwrite the number of unread bytes in the shared display list header.
* GPUProcess/graphics/DisplayListReaderHandle.h:
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
* Shared/SharedDisplayListHandle.h:
Remove this virtual method from the base class, since the reader handle now returns an `Optional<size_t>`
instead of just a `size_t`.
* WebProcess/GPU/graphics/DisplayListWriterHandle.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (271799 => 271800)
--- trunk/Source/WebKit/ChangeLog 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/ChangeLog 2021-01-25 18:33:19 UTC (rev 271800)
@@ -1,3 +1,30 @@
+2021-01-25 Wenson Hsieh <wenson_hs...@apple.com>
+
+ [GPU Process] Web process should be terminated if DisplayListReaderHandle advances past 0
+ https://bugs.webkit.org/show_bug.cgi?id=220926
+
+ Reviewed by Simon Fraser.
+
+ Address another FIXME in the GPU Process by replacing a release assertion with a MESSAGE_CHECK. See below for
+ more details.
+
+ * GPUProcess/graphics/DisplayListReaderHandle.cpp:
+ (WebKit::DisplayListReaderHandle::advance):
+
+ Return `WTF::nullopt` instead of just crashing in the case where the number of bytes to advance exceeds the
+ number of unread bytes. This can (and should) only happen in the case where a compromised web content process
+ attempts to overwrite the number of unread bytes in the shared display list header.
+
+ * GPUProcess/graphics/DisplayListReaderHandle.h:
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
+ * Shared/SharedDisplayListHandle.h:
+
+ Remove this virtual method from the base class, since the reader handle now returns an `Optional<size_t>`
+ instead of just a `size_t`.
+
+ * WebProcess/GPU/graphics/DisplayListWriterHandle.h:
+
2021-01-24 Simon Fraser <simon.fra...@apple.com>
[iOS WK2] theverge.com - rubber band scrolling at the top of the page causes an abrupt jump
Modified: trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.cpp (271799 => 271800)
--- trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.cpp 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.cpp 2021-01-25 18:33:19 UTC (rev 271800)
@@ -29,14 +29,11 @@
namespace WebKit {
using namespace WebCore;
-size_t DisplayListReaderHandle::advance(size_t amount)
+Optional<size_t> DisplayListReaderHandle::advance(size_t amount)
{
auto previousUnreadBytes = header().unreadBytes.exchangeSub(amount);
- if (UNLIKELY(previousUnreadBytes < amount)) {
- // FIXME: This should result in terminating the web process.
- RELEASE_ASSERT_NOT_REACHED();
- return 0;
- }
+ if (UNLIKELY(previousUnreadBytes < amount))
+ return WTF::nullopt;
return previousUnreadBytes - amount;
}
Modified: trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h (271799 => 271800)
--- trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/GPUProcess/graphics/DisplayListReaderHandle.h 2021-01-25 18:33:19 UTC (rev 271800)
@@ -27,6 +27,7 @@
#include "SharedDisplayListHandle.h"
#include <wtf/FastMalloc.h>
+#include <wtf/Optional.h>
namespace WebKit {
@@ -38,7 +39,7 @@
return adoptRef(*new DisplayListReaderHandle(identifier, WTFMove(sharedMemory)));
}
- size_t advance(size_t amount) override;
+ Optional<size_t> advance(size_t amount);
std::unique_ptr<WebCore::DisplayList::DisplayList> displayListForReading(size_t offset, size_t capacity, WebCore::DisplayList::ItemBufferReadingClient&) const;
void startWaiting()
Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (271799 => 271800)
--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-01-25 18:33:19 UTC (rev 271800)
@@ -194,9 +194,12 @@
MESSAGE_CHECK_WITH_RETURN_VALUE(displayList, nullptr, "Failed to map display list from shared memory");
auto result = submit(*displayList, *destination);
- sizeToRead = handle.advance(result.numberOfBytesRead);
MESSAGE_CHECK_WITH_RETURN_VALUE(result.reasonForStopping != DisplayList::StopReplayReason::InvalidItem, nullptr, "Detected invalid display list item");
+ auto advanceResult = handle.advance(result.numberOfBytesRead);
+ MESSAGE_CHECK_WITH_RETURN_VALUE(advanceResult, nullptr, "Failed to advance display list reader handle");
+ sizeToRead = *advanceResult;
+
CheckedSize checkedOffset = offset;
checkedOffset += result.numberOfBytesRead;
MESSAGE_CHECK_WITH_RETURN_VALUE(!checkedOffset.hasOverflowed(), nullptr, "Overflowed when advancing shared display list handle offset");
Modified: trunk/Source/WebKit/Shared/SharedDisplayListHandle.h (271799 => 271800)
--- trunk/Source/WebKit/Shared/SharedDisplayListHandle.h 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/Shared/SharedDisplayListHandle.h 2021-01-25 18:33:19 UTC (rev 271800)
@@ -53,8 +53,6 @@
return header().unreadBytes.load();
}
- virtual size_t advance(size_t amount) = 0;
-
enum class WaitingStatus : uint8_t {
NotWaiting,
Waiting,
Modified: trunk/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h (271799 => 271800)
--- trunk/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h 2021-01-25 18:28:41 UTC (rev 271799)
+++ trunk/Source/WebKit/WebProcess/GPU/graphics/DisplayListWriterHandle.h 2021-01-25 18:33:19 UTC (rev 271800)
@@ -43,7 +43,7 @@
bool moveWritableOffsetToStartIfPossible();
- size_t advance(size_t amount) override;
+ size_t advance(size_t amount);
WebCore::DisplayList::ItemBufferHandle createHandle() const;
bool tryToResume(SharedDisplayListHandle::ResumeReadingInformation&& info)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
