Title: [241300] trunk/Source/WebCore
Revision
241300
Author
mark....@apple.com
Date
2019-02-12 10:21:45 -0800 (Tue, 12 Feb 2019)

Log Message

Add some null checks in JSNodeCustom.h's root() and generated isReachableFromOpaqueRoots() functions.
https://bugs.webkit.org/show_bug.cgi?id=194530
<rdar://problem/47973274>

Reviewed by Chris Dumez.

This is needed to fix a null pointer dereference that arises from the following scenario:
1. a Document detaches from its StyleSheetList.
2. the JSStyleSheetList that is associated with the detached StyleSheetList has yet
   to be scanned and collected by the GC.
3. the GC eventually looks for the opaque root of the StyleSheetList's owner, and
   discovers a null owner pointer.

This patch fixes this issue by applying the following null checks:

1. Add a null check in JSNodeCustom.h's root().

   root() is called from a isReachableFromOpaqueRoots() generated by CodeGeneratorJS.pm.
   isReachableFromOpaqueRoots() calls a ownerNode() method and passes its result
   to root().  However, depending on which class the ownerNode() method belongs to,
   it can either return a pointer or a reference.  The null check only makes sense
   in the pointer case.

   To accommodate the 2 forms, root() itself is has an overload that takes a
   reference instead of a pointer.

   Since CodeGeneratorJS.pm can't tell what the generated class' ownerNode()
   returns, it can't discern when the result is a pointer and apply the null check.
   Instead, we just add the null check to the version of root() that takes a
   pointer.  If the node pointer is null, we'll return a null opaque root.

2. Fix CodeGeneratorJS.pm to null check the opaque root before using it.

* bindings/js/JSNodeCustom.h:
(WebCore::root):
* bindings/scripts/CodeGeneratorJS.pm:
(GenerateImplementation):
* bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:
(WebCore::JSTestGenerateIsReachableOwner::isReachableFromOpaqueRoots):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (241299 => 241300)


--- trunk/Source/WebCore/ChangeLog	2019-02-12 17:43:37 UTC (rev 241299)
+++ trunk/Source/WebCore/ChangeLog	2019-02-12 18:21:45 UTC (rev 241300)
@@ -1,3 +1,45 @@
+2019-02-12  Mark Lam  <mark....@apple.com>
+
+        Add some null checks in JSNodeCustom.h's root() and generated isReachableFromOpaqueRoots() functions.
+        https://bugs.webkit.org/show_bug.cgi?id=194530
+        <rdar://problem/47973274>
+
+        Reviewed by Chris Dumez.
+
+        This is needed to fix a null pointer dereference that arises from the following scenario:
+        1. a Document detaches from its StyleSheetList.
+        2. the JSStyleSheetList that is associated with the detached StyleSheetList has yet
+           to be scanned and collected by the GC.
+        3. the GC eventually looks for the opaque root of the StyleSheetList's owner, and
+           discovers a null owner pointer.
+
+        This patch fixes this issue by applying the following null checks:
+
+        1. Add a null check in JSNodeCustom.h's root().
+
+           root() is called from a isReachableFromOpaqueRoots() generated by CodeGeneratorJS.pm.
+           isReachableFromOpaqueRoots() calls a ownerNode() method and passes its result
+           to root().  However, depending on which class the ownerNode() method belongs to,
+           it can either return a pointer or a reference.  The null check only makes sense
+           in the pointer case.
+
+           To accommodate the 2 forms, root() itself is has an overload that takes a
+           reference instead of a pointer.
+
+           Since CodeGeneratorJS.pm can't tell what the generated class' ownerNode()
+           returns, it can't discern when the result is a pointer and apply the null check.
+           Instead, we just add the null check to the version of root() that takes a
+           pointer.  If the node pointer is null, we'll return a null opaque root.
+
+        2. Fix CodeGeneratorJS.pm to null check the opaque root before using it.
+
+        * bindings/js/JSNodeCustom.h:
+        (WebCore::root):
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateImplementation):
+        * bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:
+        (WebCore::JSTestGenerateIsReachableOwner::isReachableFromOpaqueRoots):
+
 2019-02-12  Andy Estes  <aes...@apple.com>
 
         [iOSMac] Enable Parental Controls Content Filtering

Modified: trunk/Source/WebCore/bindings/js/JSNodeCustom.h (241299 => 241300)


--- trunk/Source/WebCore/bindings/js/JSNodeCustom.h	2019-02-12 17:43:37 UTC (rev 241299)
+++ trunk/Source/WebCore/bindings/js/JSNodeCustom.h	2019-02-12 18:21:45 UTC (rev 241300)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2007-2019 Apple Inc. All rights reserved.
  * Copyright (C) 2018 Yusuke Suzuki <utatane....@gmail.com>.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -80,7 +80,7 @@
 
 inline void* root(Node* node)
 {
-    return node->opaqueRoot();
+    return node ? node->opaqueRoot() : nullptr;
 }
 
 inline void* root(Node& node)

Modified: trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm (241299 => 241300)


--- trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2019-02-12 17:43:37 UTC (rev 241299)
+++ trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm	2019-02-12 18:21:45 UTC (rev 241300)
@@ -3,7 +3,7 @@
 # Copyright (C) 2006 Anders Carlsson <ander...@mac.com>
 # Copyright (C) 2006, 2007 Samuel Weinig <s...@webkit.org>
 # Copyright (C) 2006 Alexey Proskuryakov <a...@webkit.org>
-# Copyright (C) 2006-2018 Apple Inc. All rights reserved.
+# Copyright (C) 2006-2019 Apple Inc. All rights reserved.
 # Copyright (C) 2009 Cameron McCormack <c...@mcc.id.au>
 # Copyright (C) Research In Motion Limited 2010. All rights reserved.
 # Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
@@ -4679,7 +4679,7 @@
             }
 
             push(@implContent, $rootString);
-            push(@implContent, "    return visitor.containsOpaqueRoot(root);\n");
+            push(@implContent, "    return root && visitor.containsOpaqueRoot(root);\n");
         } else {
             if (!$emittedJSCast) {
                 push(@implContent, "    UNUSED_PARAM(handle);\n");

Modified: trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp (241299 => 241300)


--- trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp	2019-02-12 17:43:37 UTC (rev 241299)
+++ trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp	2019-02-12 18:21:45 UTC (rev 241300)
@@ -204,7 +204,7 @@
     TestGenerateIsReachable* root = &jsTestGenerateIsReachable->wrapped();
     if (UNLIKELY(reason))
         *reason = "Reachable from TestGenerateIsReachable";
-    return visitor.containsOpaqueRoot(root);
+    return root && visitor.containsOpaqueRoot(root);
 }
 
 void JSTestGenerateIsReachableOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to