Title: [289803] trunk/Source/WebKit
- Revision
- 289803
- Author
- [email protected]
- Date
- 2022-02-15 06:06:59 -0800 (Tue, 15 Feb 2022)
Log Message
Memory for messages wrapped for testing is freed before use
https://bugs.webkit.org/show_bug.cgi?id=236590
Patch by Kimmo Kinnunen <[email protected]> on 2022-02-15
Reviewed by Antti Koivisto.
Wrapped messages are such that the data is wrapped inside a synchronous message.
These are sent by certain test objects.
In case the wrapped message was sent to another thread, the wrappee would be
destroyed before the wrapped message was read.
This is a regression from "Support in-process testing of IPC messages"
where the Decoder constructor with deallocator parameter was changed to
unconditionally mean that the Decoder owns the passed in data.
This call-site was missed, and it uses the constructor to pass nullptr
deallocator. Previously this used to mean that the constructor should
copy the data.
Fix by using the copying Decoder constructor.
Add assertion to the ownership-transfer decoder that the deallocation
function is non-null to notice the potential misuse. Currently passing
globally owned data, e.g. non-deallocated data is not useful.
Remove pessimizing WTFMove of a local variabl from a return position.
No new tests, caught by fast/events/gesture/wheel-from-gesture.html with ASAN.
* Platform/IPC/Decoder.cpp:
(IPC::Decoder::create):
(IPC::Decoder::unwrapForTesting):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (289802 => 289803)
--- trunk/Source/WebKit/ChangeLog 2022-02-15 13:10:35 UTC (rev 289802)
+++ trunk/Source/WebKit/ChangeLog 2022-02-15 14:06:59 UTC (rev 289803)
@@ -1,5 +1,36 @@
2022-02-15 Kimmo Kinnunen <[email protected]>
+ Memory for messages wrapped for testing is freed before use
+ https://bugs.webkit.org/show_bug.cgi?id=236590
+
+ Reviewed by Antti Koivisto.
+
+ Wrapped messages are such that the data is wrapped inside a synchronous message.
+ These are sent by certain test objects.
+ In case the wrapped message was sent to another thread, the wrappee would be
+ destroyed before the wrapped message was read.
+
+ This is a regression from "Support in-process testing of IPC messages"
+ where the Decoder constructor with deallocator parameter was changed to
+ unconditionally mean that the Decoder owns the passed in data.
+ This call-site was missed, and it uses the constructor to pass nullptr
+ deallocator. Previously this used to mean that the constructor should
+ copy the data.
+
+ Fix by using the copying Decoder constructor.
+ Add assertion to the ownership-transfer decoder that the deallocation
+ function is non-null to notice the potential misuse. Currently passing
+ globally owned data, e.g. non-deallocated data is not useful.
+ Remove pessimizing WTFMove of a local variabl from a return position.
+
+ No new tests, caught by fast/events/gesture/wheel-from-gesture.html with ASAN.
+
+ * Platform/IPC/Decoder.cpp:
+ (IPC::Decoder::create):
+ (IPC::Decoder::unwrapForTesting):
+
+2022-02-15 Kimmo Kinnunen <[email protected]>
+
GPUP WebGL: WTF::RefCountedBase::applyRefDerefThreadingCheck() fails due to RemoteGraphicsContextGL::paintPixelBufferToImageBuffer
https://bugs.webkit.org/show_bug.cgi?id=236501
Modified: trunk/Source/WebKit/Platform/IPC/Decoder.cpp (289802 => 289803)
--- trunk/Source/WebKit/Platform/IPC/Decoder.cpp 2022-02-15 13:10:35 UTC (rev 289802)
+++ trunk/Source/WebKit/Platform/IPC/Decoder.cpp 2022-02-15 14:06:59 UTC (rev 289803)
@@ -59,6 +59,7 @@
std::unique_ptr<Decoder> Decoder::create(const uint8_t* buffer, size_t bufferSize, BufferDeallocator&& bufferDeallocator, Vector<Attachment>&& attachments)
{
+ ASSERT(bufferDeallocator);
ASSERT(buffer);
if (UNLIKELY(!buffer)) {
RELEASE_LOG_FAULT(IPC, "Decoder::create() called with a null buffer (bufferSize: %lu)", bufferSize);
@@ -65,7 +66,9 @@
return nullptr;
}
auto decoder = std::unique_ptr<Decoder>(new Decoder(buffer, bufferSize, WTFMove(bufferDeallocator), WTFMove(attachments)));
- return decoder->isValid() ? WTFMove(decoder) : nullptr;
+ if (!decoder->isValid())
+ return nullptr;
+ return decoder;
}
Decoder::Decoder(const uint8_t* buffer, size_t bufferSize, BufferDeallocator&& bufferDeallocator, Vector<Attachment>&& attachments)
@@ -145,7 +148,7 @@
if (!decoder.decode(wrappedMessage))
return nullptr;
- return Decoder::create(wrappedMessage.data(), wrappedMessage.size(), nullptr, WTFMove(attachments));
+ return Decoder::create(wrappedMessage.data(), wrappedMessage.size(), WTFMove(attachments));
}
static inline const uint8_t* roundUpToAlignment(const uint8_t* ptr, size_t alignment)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
