- Revision
- 275334
- Author
- [email protected]
- Date
- 2021-03-31 18:39:36 -0700 (Wed, 31 Mar 2021)
Log Message
List of extents should be bounds-checked when iterating display list items
https://bugs.webkit.org/show_bug.cgi?id=224019
<rdar://problem/71851600>
Reviewed by Tim Horton.
Source/WebCore:
Add a bounds check before attempting to access the vector of display list drawing item extents. In the case
where we would've otherwise attempted to access an out-of-bounds item, we instead flag ourselves as invalid and
stop early with `StopReplayReason::InvalidItemOrExtent`.
* platform/graphics/displaylists/DisplayList.cpp:
(WebCore::DisplayList::DisplayList::setTracksDrawingItemExtents):
Drive-by fix: use `isEmpty()` in the release assertion instead of duplicating code.
(WebCore::DisplayList::DisplayList::iterator::updateCurrentDrawingItemExtent):
(WebCore::DisplayList::DisplayList::iterator::updateCurrentItem):
* platform/graphics/displaylists/DisplayList.h:
* platform/graphics/displaylists/DisplayListReplayer.cpp:
(WebCore::DisplayList::Replayer::applyItem):
(WebCore::DisplayList::Replayer::replay):
* platform/graphics/displaylists/DisplayListReplayer.h:
Rename the `StopReplayReason::InvalidItem` to `StopReplayReason::InvalidItemOrExtent`, to reflect that we may
also stop replay when encountering invalid item extents.
Source/WebKit:
Rename `InvalidItem` to `InvalidItemOrExtent`.
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
Tools:
Rename `InvalidItem` to `InvalidItemOrExtent`.
* TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (275333 => 275334)
--- trunk/Source/WebCore/ChangeLog 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebCore/ChangeLog 2021-04-01 01:39:36 UTC (rev 275334)
@@ -1,3 +1,31 @@
+2021-03-31 Wenson Hsieh <[email protected]>
+
+ List of extents should be bounds-checked when iterating display list items
+ https://bugs.webkit.org/show_bug.cgi?id=224019
+ <rdar://problem/71851600>
+
+ Reviewed by Tim Horton.
+
+ Add a bounds check before attempting to access the vector of display list drawing item extents. In the case
+ where we would've otherwise attempted to access an out-of-bounds item, we instead flag ourselves as invalid and
+ stop early with `StopReplayReason::InvalidItemOrExtent`.
+
+ * platform/graphics/displaylists/DisplayList.cpp:
+ (WebCore::DisplayList::DisplayList::setTracksDrawingItemExtents):
+
+ Drive-by fix: use `isEmpty()` in the release assertion instead of duplicating code.
+
+ (WebCore::DisplayList::DisplayList::iterator::updateCurrentDrawingItemExtent):
+ (WebCore::DisplayList::DisplayList::iterator::updateCurrentItem):
+ * platform/graphics/displaylists/DisplayList.h:
+ * platform/graphics/displaylists/DisplayListReplayer.cpp:
+ (WebCore::DisplayList::Replayer::applyItem):
+ (WebCore::DisplayList::Replayer::replay):
+ * platform/graphics/displaylists/DisplayListReplayer.h:
+
+ Rename the `StopReplayReason::InvalidItem` to `StopReplayReason::InvalidItemOrExtent`, to reflect that we may
+ also stop replay when encountering invalid item extents.
+
2021-03-31 Tadeu Zagallo <[email protected]>
Missing scope release in JSDOMBuiltinConstructorBase
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.cpp (275333 => 275334)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.cpp 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.cpp 2021-04-01 01:39:36 UTC (rev 275334)
@@ -182,7 +182,7 @@
void DisplayList::setTracksDrawingItemExtents(bool value)
{
- RELEASE_ASSERT(!m_items || m_items->isEmpty());
+ RELEASE_ASSERT(isEmpty());
m_tracksDrawingItemExtents = value;
}
@@ -332,6 +332,25 @@
return m_cursor == endCursor;
}
+DisplayList::iterator::ExtentUpdateResult DisplayList::iterator::updateCurrentDrawingItemExtent(ItemType itemType)
+{
+ auto& extents = m_displayList.m_drawingItemExtents;
+ if (extents.isEmpty())
+ return ExtentUpdateResult::Success;
+
+ if (!isDrawingItem(itemType)) {
+ m_currentExtent = WTF::nullopt;
+ return ExtentUpdateResult::Success;
+ }
+
+ if (m_drawingItemIndex >= extents.size())
+ return ExtentUpdateResult::Failure;
+
+ m_currentExtent = extents[m_drawingItemIndex];
+ m_drawingItemIndex++;
+ return ExtentUpdateResult::Success;
+}
+
void DisplayList::iterator::updateCurrentItem()
{
clearCurrentItem();
@@ -341,12 +360,12 @@
auto& items = *itemBuffer();
auto itemType = static_cast<ItemType>(m_cursor[0]);
- if (isDrawingItem(itemType) && !m_displayList.m_drawingItemExtents.isEmpty()) {
- m_currentExtent = m_displayList.m_drawingItemExtents[m_drawingItemIndex];
- m_drawingItemIndex++;
- } else
- m_currentExtent = WTF::nullopt;
+ if (updateCurrentDrawingItemExtent(itemType) == ExtentUpdateResult::Failure) {
+ m_isValid = false;
+ return;
+ }
+
auto* client = items.m_readingClient;
auto paddedSizeOfTypeAndItem = paddedSizeOfTypeAndItemInBytes(itemType);
if (client) {
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h (275333 => 275334)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayList.h 2021-04-01 01:39:36 UTC (rev 275334)
@@ -140,6 +140,10 @@
WEBCORE_EXPORT void clearCurrentItem();
WEBCORE_EXPORT void updateCurrentItem();
WEBCORE_EXPORT void advance();
+
+ enum class ExtentUpdateResult : bool { Failure, Success };
+ ExtentUpdateResult updateCurrentDrawingItemExtent(ItemType);
+
bool atEnd() const;
ItemBuffer* itemBuffer() const { return m_displayList.itemBufferIfExists(); }
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp (275333 => 275334)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.cpp 2021-04-01 01:39:36 UTC (rev 275334)
@@ -153,7 +153,7 @@
if (item.is<BeginClipToDrawingCommands>()) {
if (m_maskImageBuffer)
- return { StopReplayReason::InvalidItem, WTF::nullopt };
+ return { StopReplayReason::InvalidItemOrExtent, WTF::nullopt };
auto& clipItem = item.get<BeginClipToDrawingCommands>();
m_maskImageBuffer = ImageBuffer::createCompatibleBuffer(clipItem.destination().size(), clipItem.colorSpace(), m_context);
if (!m_maskImageBuffer)
@@ -163,7 +163,7 @@
if (item.is<EndClipToDrawingCommands>()) {
if (!m_maskImageBuffer)
- return { StopReplayReason::InvalidItem, WTF::nullopt };
+ return { StopReplayReason::InvalidItemOrExtent, WTF::nullopt };
auto& clipItem = item.get<EndClipToDrawingCommands>();
m_context.clipToImageBuffer(*m_maskImageBuffer, clipItem.destination());
m_maskImageBuffer = nullptr;
@@ -195,7 +195,7 @@
}
if (!item) {
- result.reasonForStopping = StopReplayReason::InvalidItem;
+ result.reasonForStopping = StopReplayReason::InvalidItemOrExtent;
break;
}
Modified: trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h (275333 => 275334)
--- trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebCore/platform/graphics/displaylists/DisplayListReplayer.h 2021-04-01 01:39:36 UTC (rev 275334)
@@ -42,7 +42,7 @@
ReplayedAllItems,
MissingCachedResource,
ChangeDestinationImageBuffer,
- InvalidItem,
+ InvalidItemOrExtent,
OutOfMemory
};
Modified: trunk/Source/WebKit/ChangeLog (275333 => 275334)
--- trunk/Source/WebKit/ChangeLog 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebKit/ChangeLog 2021-04-01 01:39:36 UTC (rev 275334)
@@ -1,3 +1,16 @@
+2021-03-31 Wenson Hsieh <[email protected]>
+
+ List of extents should be bounds-checked when iterating display list items
+ https://bugs.webkit.org/show_bug.cgi?id=224019
+ <rdar://problem/71851600>
+
+ Reviewed by Tim Horton.
+
+ Rename `InvalidItem` to `InvalidItemOrExtent`.
+
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
+
2021-03-31 Chris Dumez <[email protected]>
REGRESSION: The NetworkProcess fails to relaunch after it crashes
Modified: trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (275333 => 275334)
--- trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-04-01 01:39:36 UTC (rev 275334)
@@ -208,7 +208,7 @@
MESSAGE_CHECK_WITH_RETURN_VALUE(displayList, nullptr, "Failed to map display list from shared memory");
auto result = submit(*displayList, *destination);
- MESSAGE_CHECK_WITH_RETURN_VALUE(result.reasonForStopping != DisplayList::StopReplayReason::InvalidItem, nullptr, "Detected invalid display list item");
+ MESSAGE_CHECK_WITH_RETURN_VALUE(result.reasonForStopping != DisplayList::StopReplayReason::InvalidItemOrExtent, nullptr, "Detected invalid display list item or extent");
MESSAGE_CHECK_WITH_RETURN_VALUE(result.reasonForStopping != DisplayList::StopReplayReason::OutOfMemory, nullptr, "Cound not allocate memory");
auto advanceResult = handle.advance(result.numberOfBytesRead);
Modified: trunk/Tools/ChangeLog (275333 => 275334)
--- trunk/Tools/ChangeLog 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Tools/ChangeLog 2021-04-01 01:39:36 UTC (rev 275334)
@@ -1,3 +1,16 @@
+2021-03-31 Wenson Hsieh <[email protected]>
+
+ List of extents should be bounds-checked when iterating display list items
+ https://bugs.webkit.org/show_bug.cgi?id=224019
+ <rdar://problem/71851600>
+
+ Reviewed by Tim Horton.
+
+ Rename `InvalidItem` to `InvalidItemOrExtent`.
+
+ * TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
+ (TestWebKitAPI::TEST):
+
2021-03-31 BJ Burg <[email protected]>
Style checker should warn about use of future OS versions in WK_API_AVAILABLE
Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp (275333 => 275334)
--- trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-04-01 01:38:03 UTC (rev 275333)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-04-01 01:39:36 UTC (rev 275334)
@@ -132,7 +132,7 @@
EXPECT_GT(result.numberOfBytesRead, 0U);
EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
- EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItem);
+ EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent);
}
TEST(DisplayListTests, InlineItemValidationFailure)
@@ -153,7 +153,7 @@
EXPECT_EQ(result.numberOfBytesRead, 0U);
EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
- EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItem);
+ EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent);
}
} // namespace TestWebKitAPI
