Add some null checks in JSNodeCustom.h's root() and generated isReachableFromOpaqueRo...
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Feb 2019 18:21:45 +0000 (18:21 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Feb 2019 18:21:45 +0000 (18:21 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@241300 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSNodeCustom.h
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp

index 3de6940..5d1bd70 100644 (file)
@@ -1,3 +1,45 @@
+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>
 
         [iOSMac] Enable Parental Controls Content Filtering
index cb5b267..fca1b3d 100644 (file)
@@ -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.tea@gmail.com>.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -80,7 +80,7 @@ inline void willCreatePossiblyOrphanedTreeByRemoval(Node* root)
 
 inline void* root(Node* node)
 {
-    return node->opaqueRoot();
+    return node ? node->opaqueRoot() : nullptr;
 }
 
 inline void* root(Node& node)
index b53514b..54b2be0 100644 (file)
@@ -3,7 +3,7 @@
 # Copyright (C) 2006 Anders Carlsson <andersca@mac.com>
 # 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.
 # Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
@@ -4679,7 +4679,7 @@ sub GenerateImplementation
             }
 
             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");
index 9f16b82..873b617 100644 (file)
@@ -204,7 +204,7 @@ bool JSTestGenerateIsReachableOwner::isReachableFromOpaqueRoots(JSC::Handle<JSC:
     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)