Changeset 241300 in webkit

Timestamp:

Feb 12, 2019, 10:21:45 AM (6 years ago)

Author:

mark.lam@apple.com

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSNodeCustom.h (modified) (2 diffs)
• bindings/scripts/CodeGeneratorJS.pm (modified) (2 diffs)
• bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-12  Mark Lam  <mark.lam@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  <aestes@apple.com>
 

trunk/Source/WebCore/bindings/js/JSNodeCustom.h

@@ -1 +1 @@
 /*
- * 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.tea@gmail.com>.
  *
@@ -81 +81 @@
 inline void* root(Node* node)
 {
-    return node->opaqueRoot();
+    return node ? node->opaqueRoot() : nullptr;
 }
 

trunk/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm

@@ -4 +4 @@
 # Copyright (C) 2006, 2007 Samuel Weinig <sam@webkit.org>
 # Copyright (C) 2006 Alexey Proskuryakov <ap@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 <cam@mcc.id.au>
 # Copyright (C) Research In Motion Limited 2010. All rights reserved.
@@ -4680 +4680 @@
 
             push(@implContent, $rootString);
-            push(@implContent, "    return visitor.containsOpaqueRoot(root);\n");
+            push(@implContent, "    return root && visitor.containsOpaqueRoot(root);\n");
         } else {
             if (!$emittedJSCast) {

trunk/Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp

@@ -205 +205 @@
     if (UNLIKELY(reason))
         *reason = "Reachable from TestGenerateIsReachable";
-    return visitor.containsOpaqueRoot(root);
+    return root && visitor.containsOpaqueRoot(root);
 }