FuseDaemon: Fix possible double free in DeleteTree

Guarantee a parent node not be released while deleting its children.
pf_forget could be called for a parent node first not its children when
evicting file system inodes by shrinker, so the parent node could exist
without its own reference but having a children node. In this case,
this node could be deleted during executing DeleteTree(child), and it
causes double free for the node.

Bug: 307897842
Test: atest fuse_node_test
Signed-off-by: Hobin Woo <hobin.woo@samsung.corp-partner.google.com>
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:ce83d5ab64172cd6d18eba9ca66b9e7c05f206d2)
Merged-In: If4f94557035edba9d07d0fdaa08285d0799723b7
Change-Id: If4f94557035edba9d07d0fdaa08285d0799723b7

2 files changed, 24 insertions, 0 deletions

diff --git a/jni/node.cpp b/jni/node.cpp
index 31e497096..25f732d38 100644
--- a/jni/node.cpp
+++ b/jni/node.cpp
@@ -115,6 +115,14 @@ void node::DeleteTree(node* tree) {
     std::lock_guard<std::recursive_mutex> guard(*tree->lock_);
 
     if (tree) {
+        // Guarantee this node not be released while deleting its children.
+        // pf_forget could be called for a parent node first not its children
+        // when evicting file system inodes by shrinker, so the parent node
+        // could exist without its own reference but having a children node.
+        // In this case, this node could be deleted during executing
+        // DeleteTree(child), and it causes double free for the node.
+        tree->Acquire();
+
         // Make a copy of the list of children because calling Delete tree
         // will modify the list of children, which will cause issues while
         // iterating over them.
diff --git a/jni/node_test.cpp b/jni/node_test.cpp
index f687cad89..6afdd75b3 100644
--- a/jni/node_test.cpp
+++ b/jni/node_test.cpp
@@ -526,6 +526,22 @@ TEST_F(NodeTest, LookupChildByName_ChildrenWithSameName) {
     test_fn("BaZ", baz1.get(), baz2.get());
 }
 
+TEST_F(NodeTest, DestroyDoesntDoubleFree) {
+    node* root = node::Create(nullptr, "root", "", true, 0, 0, &lock_, 0, &tracker_);
+    node* child = node::Create(root, "child", "", true, 0, 0, &lock_, 0, &tracker_);
+    node* grandchild = node::Create(child, "grandchild", "", true, 0, 0, &lock_, 0, &tracker_);
+
+    // 'child' is referenced by itself and by 'grandchild'
+    ASSERT_EQ(2, GetRefCount(child));
+    // Kernel forgets about child only
+    ASSERT_FALSE(child->Release(1));
+    // Child only referenced by 'grandchild'
+    ASSERT_EQ(1, GetRefCount(child));
+
+    // Now, destroying the filesystem shouldn't result in a double free
+    node::DeleteTree(root);
+}
+
 TEST_F(NodeTest, ForChild) {
     unique_node_ptr parent = CreateNode(nullptr, "/path");
     unique_node_ptr foo1 = CreateNode(parent.get(), "FoO");