Changeset 228975 in webkit

Timestamp:

Feb 24, 2018 2:06:39 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

Null-dereference of the second argument resource of DocumentLoader::scheduleSubstituteResourceLoad
​https://bugs.webkit.org/show_bug.cgi?id=182920

Patch by Fujii Hironori <Fujii Hironori> on 2018-02-24
Reviewed by Darin Adler.

A test case
imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
always crashes due to a null-dereference if compiled and optimized
by GCC 7.2. The second argument resource of
DocumentLoader::scheduleSubstituteResourceLoad can be null if the
resource can't be found in cache. I guess GCC optimizes inline
HashMap::add based on assuming the resource never becomes null
because its type is SubstituteResource&.

This changes introduces a new method
DocumentLoader::scheduleCannotShowURLError because it looks tricky
to pass a nullptr to the second argument of
scheduleSubstituteResourceLoad.

No new tests (Covered by existing tests).

• loader/DocumentLoader.cpp:

(WebCore::DocumentLoader::scheduleCannotShowURLError): Added a new method.

• loader/DocumentLoader.h:
• loader/appcache/ApplicationCacheHost.cpp:

(WebCore::ApplicationCacheHost::maybeLoadResource):
Call scheduleCannotShowURLError if the resource not found in the appcache.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/DocumentLoader.cpp (modified) (1 diff)
• loader/DocumentLoader.h (modified) (1 diff)
• loader/appcache/ApplicationCacheHost.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-02-24  Fujii Hironori  <Hironori.Fujii@sony.com>
+
+        Null-dereference of the second argument `resource` of DocumentLoader::scheduleSubstituteResourceLoad
+        https://bugs.webkit.org/show_bug.cgi?id=182920
+
+        Reviewed by Darin Adler.
+
+        A test case
+        imported/w3c/web-platform-tests/html/browsers/offline/appcache/workers/appcache-worker.html
+        always crashes due to a null-dereference if compiled and optimized
+        by GCC 7.2. The second argument `resource` of
+        DocumentLoader::scheduleSubstituteResourceLoad can be null if the
+        resource can't be found in cache. I guess GCC optimizes inline
+        HashMap::add based on assuming the `resource` never becomes null
+        because its type is SubstituteResource&.
+
+        This changes introduces a new method
+        DocumentLoader::scheduleCannotShowURLError because it looks tricky
+        to pass a nullptr to the second argument of
+        scheduleSubstituteResourceLoad.
+
+        No new tests (Covered by existing tests).
+
+        * loader/DocumentLoader.cpp:
+        (WebCore::DocumentLoader::scheduleCannotShowURLError): Added a new method.
+        * loader/DocumentLoader.h:
+        * loader/appcache/ApplicationCacheHost.cpp:
+        (WebCore::ApplicationCacheHost::maybeLoadResource):
+        Call scheduleCannotShowURLError if the resource not found in the appcache.
+
 2018-02-17  Darin Adler  <darin@apple.com>
 

trunk/Source/WebCore/loader/DocumentLoader.cpp

@@ -1438 +1438 @@
 }
 
+void DocumentLoader::scheduleCannotShowURLError(ResourceLoader& loader)
+{
+    m_pendingSubstituteResources.set(&loader, nullptr);
+    deliverSubstituteResourcesAfterDelay();
+}
+
 void DocumentLoader::addResponse(const ResourceResponse& response)
 {

trunk/Source/WebCore/loader/DocumentLoader.h

@@ -185 +185 @@
 
     void scheduleSubstituteResourceLoad(ResourceLoader&, SubstituteResource&);
+    void scheduleCannotShowURLError(ResourceLoader&);
 
     // Return the ArchiveResource for the URL only when loading an Archive

trunk/Source/WebCore/loader/appcache/ApplicationCacheHost.cpp

@@ -183 +183 @@
         return false;
 
-    m_documentLoader.scheduleSubstituteResourceLoad(loader, *resource);
+    if (resource)
+        m_documentLoader.scheduleSubstituteResourceLoad(loader, *resource);
+    else
+        m_documentLoader.scheduleCannotShowURLError(loader);
     return true;
 }