- Revision
- 282522
- Author
- [email protected]
- Date
- 2021-09-16 02:52:40 -0700 (Thu, 16 Sep 2021)
Log Message
Merge r277631 - [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
https://bugs.webkit.org/show_bug.cgi?id=225886
rdar://78114708
Reviewed by Chris Dumez.
Source/WebCore:
Implement some stricter validation around object identifiers in when decoding display list items in the GPU
Process. Currently, we only check for the empty value (i.e. raw identifier value of 0) when iterating over these
items, but treat an identifier with the deleted value as valid; instead, we should be treating items with either
empty or deleted identifiers as invalid.
To address this, we introduce a new helper method, `ObjectIdentifier::isValid`, and turn existing checks for
`!!identifier` into `identifier.isValid()`.
Test: DisplayListTests.InlineItemValidationFailure
* platform/graphics/displaylists/DisplayListItems.h:
(WebCore::DisplayList::ClipToImageBuffer::isValid const):
(WebCore::DisplayList::DrawImageBuffer::isValid const):
(WebCore::DisplayList::DrawNativeImage::isValid const):
(WebCore::DisplayList::DrawPattern::isValid const):
(WebCore::DisplayList::PaintFrameForMedia::isValid const):
(WebCore::DisplayList::FlushContext::isValid const):
(WebCore::DisplayList::MetaCommandChangeItemBuffer::isValid const):
(WebCore::DisplayList::MetaCommandChangeDestinationImageBuffer::isValid const):
Source/WebKit:
See WebCore/ChangeLog for more details. Use `ObjectIdentifier::isValid()` instead of just checking for the empty
value, when determining whether an object identifier should trigger a message check to the web process.
* GPUProcess/graphics/RemoteRenderingBackend.cpp:
(WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
Source/WTF:
See WebCore/ChangeLog for more details. Add a helper method on `ObjectIdentifier` that returns true only if it
is the empty value or deleted value.
* wtf/ObjectIdentifier.h:
(WTF::ObjectIdentifier::isValid const):
Tools:
Adjust an existing API test to verify that the deleted object identifier value triggers an inline item decoding
failure.
* TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
(TestWebKitAPI::TEST):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.32/Source/WTF/ChangeLog (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WTF/ChangeLog 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WTF/ChangeLog 2021-09-16 09:52:40 UTC (rev 282522)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <[email protected]>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ See WebCore/ChangeLog for more details. Add a helper method on `ObjectIdentifier` that returns true only if it
+ is the empty value or deleted value.
+
+ * wtf/ObjectIdentifier.h:
+ (WTF::ObjectIdentifier::isValid const):
+
2021-04-06 Alex Christensen <[email protected]>
Add U+0581 and U+0585 to list of Armenian characters that look like Latin characters
Modified: releases/WebKitGTK/webkit-2.32/Source/WTF/wtf/ObjectIdentifier.h (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WTF/wtf/ObjectIdentifier.h 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WTF/wtf/ObjectIdentifier.h 2021-09-16 09:52:40 UTC (rev 282522)
@@ -63,6 +63,7 @@
ObjectIdentifier(HashTableDeletedValueType) : m_identifier(hashTableDeletedValue()) { }
bool isHashTableDeletedValue() const { return m_identifier == hashTableDeletedValue(); }
+ bool isValid() const { return isValidIdentifier(m_identifier); }
template<typename Encoder> void encode(Encoder& encoder) const
{
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/ChangeLog 2021-09-16 09:52:40 UTC (rev 282522)
@@ -1,5 +1,33 @@
2021-05-17 Wenson Hsieh <[email protected]>
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ Implement some stricter validation around object identifiers in when decoding display list items in the GPU
+ Process. Currently, we only check for the empty value (i.e. raw identifier value of 0) when iterating over these
+ items, but treat an identifier with the deleted value as valid; instead, we should be treating items with either
+ empty or deleted identifiers as invalid.
+
+ To address this, we introduce a new helper method, `ObjectIdentifier::isValid`, and turn existing checks for
+ `!!identifier` into `identifier.isValid()`.
+
+ Test: DisplayListTests.InlineItemValidationFailure
+
+ * platform/graphics/displaylists/DisplayListItems.h:
+ (WebCore::DisplayList::ClipToImageBuffer::isValid const):
+ (WebCore::DisplayList::DrawImageBuffer::isValid const):
+ (WebCore::DisplayList::DrawNativeImage::isValid const):
+ (WebCore::DisplayList::DrawPattern::isValid const):
+ (WebCore::DisplayList::PaintFrameForMedia::isValid const):
+ (WebCore::DisplayList::FlushContext::isValid const):
+ (WebCore::DisplayList::MetaCommandChangeItemBuffer::isValid const):
+ (WebCore::DisplayList::MetaCommandChangeDestinationImageBuffer::isValid const):
+
+2021-05-17 Wenson Hsieh <[email protected]>
+
[GPU Process] Validate DocumentMarkerLineStyle::Mode when decoding DrawDotsForDocumentMarker
https://bugs.webkit.org/show_bug.cgi?id=225874
rdar://77885775
Modified: releases/WebKitGTK/webkit-2.32/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WebCore/platform/graphics/displaylists/DisplayListItems.h 2021-09-16 09:52:40 UTC (rev 282522)
@@ -754,7 +754,7 @@
RenderingResourceIdentifier imageBufferIdentifier() const { return m_imageBufferIdentifier; }
FloatRect destinationRect() const { return m_destinationRect; }
- bool isValid() const { return !!m_imageBufferIdentifier; }
+ bool isValid() const { return m_imageBufferIdentifier.isValid(); }
void apply(GraphicsContext&, WebCore::ImageBuffer&) const;
@@ -1002,7 +1002,7 @@
FloatRect destinationRect() const { return m_destinationRect; }
ImagePaintingOptions options() const { return m_options; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageBufferIdentifier; }
+ bool isValid() const { return m_imageBufferIdentifier.isValid(); }
void apply(GraphicsContext&, WebCore::ImageBuffer&) const;
@@ -1037,7 +1037,7 @@
const FloatRect& source() const { return m_srcRect; }
const FloatRect& destinationRect() const { return m_destinationRect; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageIdentifier; }
+ bool isValid() const { return m_imageIdentifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
void apply(GraphicsContext&, NativeImage&) const;
@@ -1069,7 +1069,7 @@
FloatPoint phase() const { return m_phase; }
FloatSize spacing() const { return m_spacing; }
// FIXME: We might want to validate ImagePaintingOptions.
- bool isValid() const { return !!m_imageIdentifier; }
+ bool isValid() const { return m_imageIdentifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
void apply(GraphicsContext&, NativeImage&) const;
@@ -1987,7 +1987,7 @@
const FloatRect& destination() const { return m_destination; }
MediaPlayerIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
NO_RETURN_DUE_TO_ASSERT void apply(GraphicsContext&) const;
@@ -2220,7 +2220,7 @@
}
FlushIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
void apply(GraphicsContext&) const;
@@ -2242,7 +2242,7 @@
}
ItemBufferIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
private:
ItemBufferIdentifier m_identifier;
@@ -2260,7 +2260,7 @@
}
RenderingResourceIdentifier identifier() const { return m_identifier; }
- bool isValid() const { return !!m_identifier; }
+ bool isValid() const { return m_identifier.isValid(); }
private:
RenderingResourceIdentifier m_identifier;
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/ChangeLog 2021-09-16 09:52:40 UTC (rev 282522)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <[email protected]>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ See WebCore/ChangeLog for more details. Use `ObjectIdentifier::isValid()` instead of just checking for the empty
+ value, when determining whether an object identifier should trigger a message check to the web process.
+
+ * GPUProcess/graphics/RemoteRenderingBackend.cpp:
+ (WebKit::RemoteRenderingBackend::nextDestinationImageBufferAfterApplyingDisplayLists):
+
2021-05-12 Julian Gonzalez <[email protected]>
Crash in WebPageProxy::endColorPicker()
Modified: releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Source/WebKit/GPUProcess/graphics/RemoteRenderingBackend.cpp 2021-09-16 09:52:40 UTC (rev 282522)
@@ -252,7 +252,7 @@
MESSAGE_CHECK_WITH_RETURN_VALUE(sizeToRead, nullptr, "No unread bytes when resuming display list processing");
auto newDestinationIdentifier = makeObjectIdentifier<RenderingResourceIdentifierType>(resumeReadingInfo->destination);
- MESSAGE_CHECK_WITH_RETURN_VALUE(newDestinationIdentifier, nullptr, "Invalid image buffer destination when resuming display list processing");
+ MESSAGE_CHECK_WITH_RETURN_VALUE(newDestinationIdentifier.isValid(), nullptr, "Invalid image buffer destination when resuming display list processing");
destination = makeRefPtr(m_remoteResourceCache.cachedImageBuffer(newDestinationIdentifier));
MESSAGE_CHECK_WITH_RETURN_VALUE(destination, nullptr, "Missing image buffer destination when resuming display list processing");
Modified: releases/WebKitGTK/webkit-2.32/Tools/ChangeLog (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Tools/ChangeLog 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Tools/ChangeLog 2021-09-16 09:52:40 UTC (rev 282522)
@@ -1,3 +1,17 @@
+2021-05-17 Wenson Hsieh <[email protected]>
+
+ [GPU Process] Object identifiers with the deleted value should cause MESSAGE_CHECKs
+ https://bugs.webkit.org/show_bug.cgi?id=225886
+ rdar://78114708
+
+ Reviewed by Chris Dumez.
+
+ Adjust an existing API test to verify that the deleted object identifier value triggers an inline item decoding
+ failure.
+
+ * TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp:
+ (TestWebKitAPI::TEST):
+
2021-04-06 Alex Christensen <[email protected]>
Add U+0581 and U+0585 to list of Armenian characters that look like Latin characters
Modified: releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp (282521 => 282522)
--- releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-09-16 09:52:29 UTC (rev 282521)
+++ releases/WebKitGTK/webkit-2.32/Tools/TestWebKitAPI/Tests/WebCore/cg/DisplayListTestsCG.cpp 2021-09-16 09:52:40 UTC (rev 282522)
@@ -143,15 +143,24 @@
auto cgContext = adoptCF(CGBitmapContextCreate(nullptr, contextWidth, contextHeight, 8, 4 * contextWidth, colorSpace.get(), kCGImageAlphaPremultipliedLast));
GraphicsContext context { cgContext.get() };
- DisplayList list;
- list.append<FlushContext>(FlushIdentifier { });
+ auto runTestWithInvalidIdentifier = [&](FlushIdentifier identifier) {
+ EXPECT_FALSE(identifier.isValid());
- Replayer replayer { context, list };
- auto result = replayer.replay();
- EXPECT_EQ(result.numberOfBytesRead, 0U);
- EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
- EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
- EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItem);
+ DisplayList list;
+ ReadingClient reader;
+ list.setItemBufferReadingClient(&reader);
+ list.append<FlushContext>(identifier);
+
+ Replayer replayer { context, list };
+ auto result = replayer.replay();
+ EXPECT_EQ(result.numberOfBytesRead, 0U);
+ EXPECT_EQ(result.nextDestinationImageBuffer, WTF::nullopt);
+ EXPECT_EQ(result.missingCachedResourceIdentifier, WTF::nullopt);
+ EXPECT_EQ(result.reasonForStopping, StopReplayReason::InvalidItemOrExtent);
+ };
+
+ runTestWithInvalidIdentifier(FlushIdentifier { });
+ runTestWithInvalidIdentifier(FlushIdentifier { WTF::HashTableDeletedValue });
}
} // namespace TestWebKitAPI
