Repeat proxy limit callbacks if we keep going above the limit.

When we hit the high watermark for number of live proxies in a process,
we call a callback with the offending UID. ActivityManagerService will
then kill all processes belonging to this UID. The proxy limit callback
is only "re-armed" when the number of proxies for that UID drops below a
certain value again. However, because the proxies beloging to the now
dead processes will only be removed when the GC next runs, and restarted
processes may start adding new proxies in the mean time, we may actually
never drop below the lower threshold again, and never fire the callback
again, despite an ever-increasing amount of proxies.

To prevent this, repeatedly fire the limit callback for every 1000
proxies that we go over the limit. This, in combination with a GC in the
framework, should make sure that we regularly kill and clean up these
processes.

Bug: 198340142
Test: Manual
Change-Id: I8d877d409863fdae9cffd43709b647d0c1edb440

2 files changed, 21 insertions, 2 deletions

diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 55566e2db9..1100d72a0c 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -36,7 +36,8 @@ namespace android {
 // ---------------------------------------------------------------------------
 
 Mutex BpBinder::sTrackingLock;
-std::unordered_map<int32_t,uint32_t> BpBinder::sTrackingMap;
+std::unordered_map<int32_t, uint32_t> BpBinder::sTrackingMap;
+std::unordered_map<int32_t, uint32_t> BpBinder::sLastLimitCallbackMap;
 int BpBinder::sNumTrackedUids = 0;
 std::atomic_bool BpBinder::sCountByUidEnabled(false);
 binder_proxy_limit_callback BpBinder::sLimitCallback;
@@ -47,6 +48,10 @@ uint32_t BpBinder::sBinderProxyCountHighWatermark = 2500;
 // Another arbitrary value a binder count needs to drop below before another callback will be called
 uint32_t BpBinder::sBinderProxyCountLowWatermark = 2000;
 
+// Once the limit has been exceeded, keep calling the limit callback for every this many new proxies
+// created over the limit.
+constexpr uint32_t REPEAT_LIMIT_CALLBACK_INTERVAL = 1000;
+
 enum {
     LIMIT_REACHED_MASK = 0x80000000,        // A flag denoting that the limit has been reached
     COUNTING_VALUE_MASK = 0x7FFFFFFF,       // A mask of the remaining bits for the count value
@@ -120,12 +125,24 @@ sp<BpBinder> BpBinder::create(int32_t handle) {
             if (sBinderProxyThrottleCreate) {
                 return nullptr;
             }
+            trackedValue = trackedValue & COUNTING_VALUE_MASK;
+            uint32_t lastLimitCallbackAt = sLastLimitCallbackMap[trackedUid];
+
+            if (trackedValue > lastLimitCallbackAt &&
+                (trackedValue - lastLimitCallbackAt > REPEAT_LIMIT_CALLBACK_INTERVAL)) {
+                ALOGE("Still too many binder proxy objects sent to uid %d from uid %d (%d proxies "
+                      "held)",
+                      getuid(), trackedUid, trackedValue);
+                if (sLimitCallback) sLimitCallback(trackedUid);
+                sLastLimitCallbackMap[trackedUid] = trackedValue;
+            }
         } else {
             if ((trackedValue & COUNTING_VALUE_MASK) >= sBinderProxyCountHighWatermark) {
                 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);
+                sLastLimitCallbackMap[trackedUid] = trackedValue & COUNTING_VALUE_MASK;
                 if (sBinderProxyThrottleCreate) {
                     ALOGI("Throttling binder proxy creates from uid %d in uid %d until binder proxy"
                           " count drops below %d",
@@ -465,8 +482,9 @@ BpBinder::~BpBinder()
                 ((trackedValue & COUNTING_VALUE_MASK) <= sBinderProxyCountLowWatermark)
                 )) {
                 ALOGI("Limit reached bit reset for uid %d (fewer than %d proxies from uid %d held)",
-                                   getuid(), mTrackedUid, sBinderProxyCountLowWatermark);
+                      getuid(), sBinderProxyCountLowWatermark, mTrackedUid);
                 sTrackingMap[mTrackedUid] &= ~LIMIT_REACHED_MASK;
+                sLastLimitCallbackMap.erase(mTrackedUid);
             }
             if (--sTrackingMap[mTrackedUid] == 0) {
                 sTrackingMap.erase(mTrackedUid);
diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index a6d35c7481..b58cb7e69a 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -194,6 +194,7 @@ private:
     static uint32_t                             sBinderProxyCountHighWatermark;
     static uint32_t                             sBinderProxyCountLowWatermark;
     static bool                                 sBinderProxyThrottleCreate;
+    static std::unordered_map<int32_t,uint32_t> sLastLimitCallbackMap;
 };
 
 } // namespace android