Changeset 282241 in webkit

Timestamp:

Sep 9, 2021, 3:24:42 PM (5 years ago)

Author:

Chris Dumez

Message:

Potential crash under CachedRawResource::didAddClient()
​https://bugs.webkit.org/show_bug.cgi?id=230121
<rdar://82936913>

Reviewed by Alex Christensen.

In r280083, I tried to address this crash by holding a strong reference
to the SharedBuffer before calling forEachSegment() on it. However, the
crash is still happening after this fix.

My suspicion is that the SharedBuffer's internal m_segments vector gets
modified as we iterate over it. As a result, I am reverting r280083 and
iterating over a copy of m_segments in forEachSegment() instead.

No new tests, we haven't been able to reproduce.

• WebCore.xcodeproj/project.pbxproj:
• loader/cache/CachedRawResource.cpp:

(WebCore::CachedRawResource::didAddClient):

• platform/SharedBuffer.cpp:

(WebCore::SharedBuffer::forEachSegment const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/cache/CachedRawResource.cpp (modified) (1 diff)
• platform/SharedBuffer.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-09-09  Chris Dumez  <cdumez@apple.com>
+
+        Potential crash under CachedRawResource::didAddClient()
+        https://bugs.webkit.org/show_bug.cgi?id=230121
+        <rdar://82936913>
+
+        Reviewed by Alex Christensen.
+
+        In r280083, I tried to address this crash by holding a strong reference
+        to the SharedBuffer before calling forEachSegment() on it. However, the
+        crash is still happening after this fix.
+
+        My suspicion is that the SharedBuffer's internal m_segments vector gets
+        modified as we iterate over it. As a result, I am reverting r280083 and
+        iterating over a copy of m_segments in forEachSegment() instead.
+
+        No new tests, we haven't been able to reproduce.
+
+        * WebCore.xcodeproj/project.pbxproj:
+        * loader/cache/CachedRawResource.cpp:
+        (WebCore::CachedRawResource::didAddClient):
+        * platform/SharedBuffer.cpp:
+        (WebCore::SharedBuffer::forEachSegment const):
+
 2021-09-09  Fujii Hironori  <Hironori.Fujii@sony.com>
 

trunk/Source/WebCore/loader/cache/CachedRawResource.cpp

@@ -168 +168 @@
             if (!hasClient(*client))
                 return;
-            if (auto data = m_data) {
-                data->forEachSegment([&](auto& segment) {
+            if (m_data) {
+                m_data->forEachSegment([&](auto& segment) {
                     if (hasClient(*client))
                         client->dataReceived(*this, segment.data(), segment.size());

trunk/Source/WebCore/platform/SharedBuffer.cpp

@@ -239 +239 @@
 void SharedBuffer::forEachSegment(const Function<void(const Span<const uint8_t>&)>& apply) const
 {
-    for (auto& segment : m_segments)
+    auto segments = m_segments;
+    for (auto& segment : segments)
         apply(Span { segment.segment->data(), segment.segment->size() });
 }