From 2e28bb237285d783d0e134275c7803d0a2f369d1 Mon Sep 17 00:00:00 2001
From: "hyeeun.jun@samsung.com" <hyeeun.jun@samsung.com>
Date: Wed, 29 Jan 2020 17:09:06 +0900
Subject: [PATCH] Fix Deadlock Issue On AppFuseBridge

There are two locks used by AppFuseBridge.
First is it's object lock, and the second is a mutex lock in app fuse library.
There are two oppsite routines to get those locks.

  (Thread A) Got Java lock -> Try to get Native lock
  (Thread B)        Got Native lock -> Try to get Java lock

Bug : https://issuetracker.google.com/issues/145707568
Signed-off-by: hyeeun.jun@samsung.com <hyeeun.jun@samsung.com>

The order must be followed to obtain two locks.
If not, the dead lock will be caused.
Therefore we change the routine to get the mutex lock first, and the object lock later.
---
 .../android/server/storage/AppFuseBridge.java  | 12 ++++++++++++
 ...om_android_server_storage_AppFuseBridge.cpp | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/services/core/java/com/android/server/storage/AppFuseBridge.java b/services/core/java/com/android/server/storage/AppFuseBridge.java
index 9d6a64701e855..b00540fd2a529 100644
--- a/services/core/java/com/android/server/storage/AppFuseBridge.java
+++ b/services/core/java/com/android/server/storage/AppFuseBridge.java
@@ -56,6 +56,15 @@ public class AppFuseBridge implements Runnable {
 
     public ParcelFileDescriptor addBridge(MountScope mountScope)
             throws FuseUnavailableMountException, NativeDaemonConnectorException {
+        /*
+        ** Dead Lock between Java lock (AppFuseBridge.java) and Native lock (FuseBridgeLoop.cc)
+        **
+        **  (Thread A) Got Java lock (addBrdige) -> Try to get Native lock (native_add_brdige)
+        **  (Thread B)        Got Native lock (FuseBrdigeLoop.start) -> Try to get Java lock (onClosed)
+        **
+        ** Guarantee the lock order (native lock -> java lock) when adding Bridge.
+        */
+        native_lock();
         try {
             synchronized (this) {
                 Preconditions.checkArgument(mScopes.indexOfKey(mountScope.mountId) < 0);
@@ -73,6 +82,7 @@ public class AppFuseBridge implements Runnable {
                 return result;
             }
         } finally {
+            native_unlock();
             IoUtils.closeQuietly(mountScope);
         }
     }
@@ -159,4 +169,6 @@ public class AppFuseBridge implements Runnable {
     private native void native_delete(long loop);
     private native void native_start_loop(long loop);
     private native int native_add_bridge(long loop, int mountId, int deviceId);
+    private native void native_lock();
+    private native void native_unlock();
 }
diff --git a/services/core/jni/com_android_server_storage_AppFuseBridge.cpp b/services/core/jni/com_android_server_storage_AppFuseBridge.cpp
index e51963340ae14..20210abd0ea89 100644
--- a/services/core/jni/com_android_server_storage_AppFuseBridge.cpp
+++ b/services/core/jni/com_android_server_storage_AppFuseBridge.cpp
@@ -123,6 +123,14 @@ jint com_android_server_storage_AppFuseBridge_add_bridge(
     return proxyFd[1].release();
 }
 
+void com_android_server_storage_AppFuseBridge_lock(JNIEnv* env, jobject self) {
+    fuse::FuseBridgeLoop::Lock();
+}
+
+void com_android_server_storage_AppFuseBridge_unlock(JNIEnv* env, jobject self) {
+    fuse::FuseBridgeLoop::Unlock();
+}
+
 const JNINativeMethod methods[] = {
     {
         "native_new",
@@ -143,6 +151,16 @@ const JNINativeMethod methods[] = {
         "native_add_bridge",
         "(JII)I",
         reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_add_bridge)
+    },
+    {
+        "native_lock",
+        "()V",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_lock)
+    },
+    {
+        "native_unlock",
+        "()V",
+        reinterpret_cast<void*>(com_android_server_storage_AppFuseBridge_unlock)
     }
 };