commit 7c166e19051768edfa7c098c8be1f635f429f819
author Steven Moreland <smoreland@google.com> Sat Jul 20 00:40:56 2024 +0000
committer Steven Moreland <smoreland@google.com> Sat Jul 20 00:46:44 2024 +0000
tree ead2e2b075c2778d15b1e1ba45fa8862015453cc
parent e2f79abef85d9dfb3ffcc09e0f94c82607f16cf7

libbinder: don't hold global locks for callbacks..

... that can be set by other libraries :)

Bug: 354286280
Bug: 199683153
Bug: 352692435
Test: boot
Change-Id: I8b8b9a243336a45af50fffbddcab13808a4a1bdc

libs/binder/BpBinder.cpp

diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 6594aa6..af9e04b 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -160,11 +160,12 @@
 
 // ---------------------------------------------------------------------------
 
-sp<BpBinder> BpBinder::create(int32_t handle) {
+sp<BpBinder> BpBinder::create(int32_t handle, std::function<void()>* postTask) {
     if constexpr (!kEnableKernelIpc) {
         LOG_ALWAYS_FATAL("Binder kernel driver disabled at build time");
         return nullptr;
     }
+    LOG_ALWAYS_FATAL_IF(postTask == nullptr, "BAD STATE");
 
     int32_t trackedUid = -1;
     if (sCountByUidEnabled) {
@@ -183,7 +184,11 @@
                 ALOGE("Still too many binder proxy objects sent to uid %d from uid %d (%d proxies "
                       "held)",
                       getuid(), trackedUid, trackedValue);
-                if (sLimitCallback) sLimitCallback(trackedUid);
+
+                if (sLimitCallback) {
+                    *postTask = [=]() { sLimitCallback(trackedUid); };
+                }
+
                 sLastLimitCallbackMap[trackedUid] = trackedValue;
             }
         } else {
@@ -197,7 +202,11 @@
                 ALOGE("Too many binder proxy objects sent to uid %d from uid %d (%d proxies held)",
                       getuid(), trackedUid, trackedValue);
                 sTrackingMap[trackedUid] |= LIMIT_REACHED_MASK;
-                if (sLimitCallback) sLimitCallback(trackedUid);
+
+                if (sLimitCallback) {
+                    *postTask = [=]() { sLimitCallback(trackedUid); };
+                }
+
                 sLastLimitCallbackMap[trackedUid] = trackedValue & COUNTING_VALUE_MASK;
                 if (sBinderProxyThrottleCreate) {
                     ALOGI("Throttling binder proxy creates from uid %d in uid %d until binder proxy"

libs/binder/ProcessState.cpp

diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp
index a42ede2..29ad8ef 100644
--- a/libs/binder/ProcessState.cpp
+++ b/libs/binder/ProcessState.cpp
@@ -311,6 +311,7 @@
 sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle)
 {
     sp<IBinder> result;
+    std::function<void()> postTask;
 
     std::unique_lock<std::mutex> _l(mLock);
 
@@ -358,7 +359,7 @@
                    return nullptr;
             }
 
-            sp<BpBinder> b = BpBinder::PrivateAccessor::create(handle);
+            sp<BpBinder> b = BpBinder::PrivateAccessor::create(handle, &postTask);
             e->binder = b.get();
             if (b) e->refs = b->getWeakRefs();
             result = b;
@@ -371,6 +372,10 @@
         }
     }
 
+    _l.unlock();
+
+    if (postTask) postTask();
+
     return result;
 }
 

libs/binder/include/binder/BpBinder.h

diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index d7f74c4..ca879a8 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -134,7 +134,9 @@
         friend class ::android::RpcState;
         explicit PrivateAccessor(const BpBinder* binder) : mBinder(binder) {}
 
-        static sp<BpBinder> create(int32_t handle) { return BpBinder::create(handle); }
+        static sp<BpBinder> create(int32_t handle, std::function<void()>* postTask) {
+            return BpBinder::create(handle, postTask);
+        }
         static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address) {
             return BpBinder::create(session, address);
         }
@@ -156,7 +158,7 @@
     friend PrivateAccessor;
     friend class sp<BpBinder>;
 
-    static sp<BpBinder> create(int32_t handle);
+    static sp<BpBinder> create(int32_t handle, std::function<void()>* postTask);
     static sp<BpBinder> create(const sp<RpcSession>& session, uint64_t address);
 
     struct BinderHandle {