Browse Source

chore: cherry-pick fix from chromium issue 1065122 (#24555)

Cheng Zhao 4 years ago
parent
commit
ef805d7d3b
2 changed files with 99 additions and 0 deletions
  1. 1 0
      patches/chromium/.patches
  2. 98 0
      patches/chromium/backport_1065122.patch

+ 1 - 0
patches/chromium/.patches

@@ -127,3 +127,4 @@ fix_default_to_ntlm_v2_in_network_service.patch
 fix_allow_ime_to_insert_zero-length_composition_string.patch
 fix_handling_non_client_pointer_events_from_pen_on_windows_10.patch
 backport_1063177.patch
+backport_1065122.patch

+ 98 - 0
patches/chromium/backport_1065122.patch

@@ -0,0 +1,98 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Cheng Zhao <[email protected]>
+Date: Thu, 4 Oct 2018 14:57:02 -0700
+Subject: fix: add some crash debugging checks
+
+[1065122] [High]: heap-use-after-free : ui::AXTreeSerializerblink::WebAXObject,content::AXContentNodeData,content::AXContentTreeData::LeastCommonAncestor
+Backport https://chromium.googlesource.com/chromium/src/+/e0a661af6c2c7346183c47b106afecb01adbfa2e
+
+diff --git a/ui/accessibility/ax_tree_serializer.h b/ui/accessibility/ax_tree_serializer.h
+index d0cd8be8dbf3c15ce2a2c84446f5042819686b9a..8791c58789cef10a31799d57287860ba8e60af94 100644
+--- a/ui/accessibility/ax_tree_serializer.h
++++ b/ui/accessibility/ax_tree_serializer.h
+@@ -180,6 +180,8 @@ class AXTreeSerializer {
+   // like when Reset() is called.
+   void InternalReset();
+ 
++  ClientTreeNode* GetClientTreeNodeParent(ClientTreeNode* obj);
++
+   // The tree source.
+   AXTreeSource<AXSourceNode, AXNodeData, AXTreeData>* tree_;
+ 
+@@ -269,7 +271,7 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::LeastCommonAncestor(
+   std::vector<ClientTreeNode*> client_ancestors;
+   while (client_node) {
+     client_ancestors.push_back(client_node);
+-    client_node = client_node->parent;
++    client_node = GetClientTreeNodeParent(client_node);
+   }
+ 
+   // Start at the root. Keep going until the source ancestor chain and
+@@ -304,9 +306,12 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::LeastCommonAncestor(
+   // that we're inside of an invalid subtree that all needs to be
+   // re-serialized, so the LCA should be higher.
+   ClientTreeNode* client_node = ClientTreeNodeById(tree_->GetId(node));
+-  while (
+-      tree_->IsValid(node) &&
+-      (!client_node || (client_node->parent && client_node->parent->invalid))) {
++  while (tree_->IsValid(node)) {
++    if (client_node) {
++      ClientTreeNode* parent = GetClientTreeNodeParent(client_node);
++      if (!parent || !parent->invalid)
++        break;
++    }
+     node = tree_->GetParent(node);
+     if (tree_->IsValid(node))
+       client_node = ClientTreeNodeById(tree_->GetId(node));
+@@ -326,12 +331,13 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
+     int child_id = tree_->GetId(child);
+     ClientTreeNode* client_child = ClientTreeNodeById(child_id);
+     if (client_child) {
+-      if (!client_child->parent) {
++      ClientTreeNode* parent = client_child->parent;
++      if (!parent) {
+         // If the client child has no parent, it must have been the
+         // previous root node, so there is no LCA and we can exit early.
+         *out_lca = tree_->GetNull();
+         return true;
+-      } else if (client_child->parent->id != id) {
++      } else if (parent->id != id) {
+         // If the client child's parent is not this node, update the LCA
+         // and return true (reparenting was found).
+         *out_lca = LeastCommonAncestor(*out_lca, client_child);
+@@ -362,6 +368,19 @@ AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::ClientTreeNodeById(
+   return nullptr;
+ }
+ 
++template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
++ClientTreeNode*
++AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::GetClientTreeNodeParent(
++    ClientTreeNode* obj) {
++  ClientTreeNode* parent = obj->parent;
++#if DCHECK_IS_ON()
++  if (!parent)
++    return nullptr;
++  DCHECK(ClientTreeNodeById(parent->id)) << "Parent not in id map.";
++#endif  // DCHECK_IS_ON()
++  return parent;
++}
++
+ template <typename AXSourceNode, typename AXNodeData, typename AXTreeData>
+ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::SerializeChanges(
+     AXSourceNode node,
+@@ -545,8 +564,13 @@ bool AXTreeSerializer<AXSourceNode, AXNodeData, AXTreeData>::
+     // above. If this happens, reset and return an error.
+ 
+     ClientTreeNode* client_child = ClientTreeNodeById(new_child_id);
+-    if (client_child && client_child->parent != client_node) {
+-      DVLOG(1) << "Reparenting detected";
++    if (client_child && GetClientTreeNodeParent(client_child) != client_node) {
++      DVLOG(1) << "Illegal reparenting detected";
++#if defined(ADDRESS_SANITIZER)
++      // Wrapping this in ADDRESS_SANITIZER will cause it to run on
++      // clusterfuzz, which should help us narrow down the issue.
++      NOTREACHED() << "Illegal reparenting detected";
++#endif
+       Reset();
+       return false;
+     }