From e48baa5fafcb127623be013c5404b559b79422da Mon Sep 17 00:00:00 2001
From: Martijn Coenen <maco@google.com>
Date: Wed, 9 Oct 2019 11:10:01 +0200
Subject: Don't hold sProxyLock while retrieving interface descriptors.

Since we can't guarantee apps won't call back into us while calling
getInterfaceDescriptor() (even though that's a *really* bad idea),
don't hold any locks while doing so, to protect us from deadlocks.

Bug: 140603195
Test: adb shell am dump binder-proxies
Change-Id: I6289c799aa7027f80792cb751fe1dc437552ea58
---
 core/java/android/os/BinderProxy.java | 59 ++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 29 deletions(-)

diff --git a/core/java/android/os/BinderProxy.java b/core/java/android/os/BinderProxy.java
index 97c0a13e7a2b..c35b05f10498 100644
--- a/core/java/android/os/BinderProxy.java
+++ b/core/java/android/os/BinderProxy.java
@@ -241,31 +241,36 @@ public final class BinderProxy implements IBinder {
             }
 
             Map<String, Integer> counts = new HashMap<>();
-            for (ArrayList<WeakReference<BinderProxy>> a : mMainIndexValues) {
-                if (a != null) {
-                    for (WeakReference<BinderProxy> weakRef : a) {
-                        BinderProxy bp = weakRef.get();
-                        String key;
-                        if (bp == null) {
-                            key = "<cleared weak-ref>";
-                        } else {
-                            try {
-                                key = bp.getInterfaceDescriptor();
-                                if ((key == null || key.isEmpty()) && !bp.isBinderAlive()) {
-                                    key = "<proxy to dead node>";
-                                }
-                            } catch (Throwable t) {
-                                key = "<exception during getDescriptor>";
-                            }
-                        }
-                        Integer i = counts.get(key);
-                        if (i == null) {
-                            counts.put(key, 1);
-                        } else {
-                            counts.put(key, i + 1);
+            final ArrayList<WeakReference<BinderProxy>> proxiesToQuery =
+                    new ArrayList<WeakReference<BinderProxy>>();
+            synchronized (sProxyMap) {
+                for (ArrayList<WeakReference<BinderProxy>> a : mMainIndexValues) {
+                    if (a != null) {
+                        proxiesToQuery.addAll(a);
+                    }
+                }
+            }
+            for (WeakReference<BinderProxy> weakRef : proxiesToQuery) {
+                BinderProxy bp = weakRef.get();
+                String key;
+                if (bp == null) {
+                    key = "<cleared weak-ref>";
+                } else {
+                    try {
+                        key = bp.getInterfaceDescriptor();
+                        if ((key == null || key.isEmpty()) && !bp.isBinderAlive()) {
+                            key = "<proxy to dead node>";
                         }
+                    } catch (Throwable t) {
+                        key = "<exception during getDescriptor>";
                     }
                 }
+                Integer i = counts.get(key);
+                if (i == null) {
+                    counts.put(key, 1);
+                } else {
+                    counts.put(key, i + 1);
+                }
             }
             Map.Entry<String, Integer>[] sorted = counts.entrySet().toArray(
                     new Map.Entry[counts.size()]);
@@ -354,9 +359,7 @@ public final class BinderProxy implements IBinder {
      * @hide
      */
     public static InterfaceCount[] getSortedInterfaceCounts(int num) {
-        synchronized (sProxyMap) {
-            return sProxyMap.getSortedInterfaceCounts(num);
-        }
+        return sProxyMap.getSortedInterfaceCounts(num);
     }
 
     /**
@@ -376,10 +379,8 @@ public final class BinderProxy implements IBinder {
      */
     public static void dumpProxyDebugInfo() {
         if (Build.IS_DEBUGGABLE) {
-            synchronized (sProxyMap) {
-                sProxyMap.dumpProxyInterfaceCounts();
-                sProxyMap.dumpPerUidProxyCounts();
-            }
+            sProxyMap.dumpProxyInterfaceCounts();
+            sProxyMap.dumpPerUidProxyCounts();
         }
     }
 
-- 
cgit v1.2.3-59-g8ed1b