Merge "Add a new callback for the excessive binder proxy count" into main

4 files changed, 117 insertions, 13 deletions

diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 42dd6916c7..54457fc568 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -44,6 +44,7 @@ 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;
+binder_proxy_warning_callback BpBinder::sWarningCallback;
 bool BpBinder::sBinderProxyThrottleCreate = false;
 
 static StaticString16 kDescriptorUninit(u"");
@@ -52,6 +53,9 @@ static StaticString16 kDescriptorUninit(u"");
 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;
+// Arbitrary value between low and high watermark on a bad behaving app to
+// trigger a warning callback.
+uint32_t BpBinder::sBinderProxyCountWarningWatermark = 2250;
 
 std::atomic<uint32_t> BpBinder::sBinderProxyCount(0);
 std::atomic<uint32_t> BpBinder::sBinderProxyCountWarned(0);
@@ -63,7 +67,8 @@ static constexpr uint32_t kBinderProxyCountWarnInterval = 5000;
 
 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
+    WARNING_REACHED_MASK = 0x40000000,      // A flag denoting that the warning has been reached
+    COUNTING_VALUE_MASK = 0x3FFFFFFF,       // A mask of the remaining bits for the count value
 };
 
 BpBinder::ObjectManager::ObjectManager()
@@ -181,7 +186,13 @@ sp<BpBinder> BpBinder::create(int32_t handle) {
                 sLastLimitCallbackMap[trackedUid] = trackedValue;
             }
         } else {
-            if ((trackedValue & COUNTING_VALUE_MASK) >= sBinderProxyCountHighWatermark) {
+            uint32_t currentValue = trackedValue & COUNTING_VALUE_MASK;
+            if (currentValue >= sBinderProxyCountWarningWatermark
+                    && currentValue < sBinderProxyCountHighWatermark
+                    && ((trackedValue & WARNING_REACHED_MASK) == 0)) [[unlikely]] {
+                sTrackingMap[trackedUid] |= WARNING_REACHED_MASK;
+                if (sWarningCallback) sWarningCallback(trackedUid);
+            } else if (currentValue >= 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;
@@ -609,11 +620,11 @@ BpBinder::~BpBinder() {
                   binderHandle());
         } else {
             auto countingValue = trackedValue & COUNTING_VALUE_MASK;
-            if ((trackedValue & LIMIT_REACHED_MASK) &&
+            if ((trackedValue & (LIMIT_REACHED_MASK | WARNING_REACHED_MASK)) &&
                 (countingValue <= sBinderProxyCountLowWatermark)) [[unlikely]] {
                 ALOGI("Limit reached bit reset for uid %d (fewer than %d proxies from uid %d held)",
                       getuid(), sBinderProxyCountLowWatermark, mTrackedUid);
-                sTrackingMap[mTrackedUid] &= ~LIMIT_REACHED_MASK;
+                sTrackingMap[mTrackedUid] &= ~(LIMIT_REACHED_MASK | WARNING_REACHED_MASK);
                 sLastLimitCallbackMap.erase(mTrackedUid);
             }
             if (--sTrackingMap[mTrackedUid] == 0) {
@@ -730,15 +741,18 @@ void BpBinder::enableCountByUid() { sCountByUidEnabled.store(true); }
 void BpBinder::disableCountByUid() { sCountByUidEnabled.store(false); }
 void BpBinder::setCountByUidEnabled(bool enable) { sCountByUidEnabled.store(enable); }
 
-void BpBinder::setLimitCallback(binder_proxy_limit_callback cb) {
+void BpBinder::setBinderProxyCountEventCallback(binder_proxy_limit_callback cbl,
+                                                binder_proxy_warning_callback cbw) {
     RpcMutexUniqueLock _l(sTrackingLock);
-    sLimitCallback = cb;
+    sLimitCallback = std::move(cbl);
+    sWarningCallback = std::move(cbw);
 }
 
-void BpBinder::setBinderProxyCountWatermarks(int high, int low) {
+void BpBinder::setBinderProxyCountWatermarks(int high, int low, int warning) {
     RpcMutexUniqueLock _l(sTrackingLock);
     sBinderProxyCountHighWatermark = high;
     sBinderProxyCountLowWatermark = low;
+    sBinderProxyCountWarningWatermark = warning;
 }
 
 // ---------------------------------------------------------------------------
diff --git a/libs/binder/include/binder/BpBinder.h b/libs/binder/include/binder/BpBinder.h
index 89a4d273ad..9f0390769b 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -35,7 +35,8 @@ class Stability;
 }
 class ProcessState;
 
-using binder_proxy_limit_callback = void(*)(int);
+using binder_proxy_limit_callback = std::function<void(int)>;
+using binder_proxy_warning_callback = std::function<void(int)>;
 
 class BpBinder : public IBinder
 {
@@ -86,8 +87,9 @@ public:
     static void         enableCountByUid();
     static void         disableCountByUid();
     static void         setCountByUidEnabled(bool enable);
-    static void         setLimitCallback(binder_proxy_limit_callback cb);
-    static void         setBinderProxyCountWatermarks(int high, int low);
+    static void         setBinderProxyCountEventCallback(binder_proxy_limit_callback cbl,
+                                                         binder_proxy_warning_callback cbw);
+    static void         setBinderProxyCountWatermarks(int high, int low, int warning);
     static uint32_t     getBinderProxyCount();
 
     std::optional<int32_t> getDebugBinderHandle() const;
@@ -212,6 +214,8 @@ private:
     static std::unordered_map<int32_t,uint32_t> sLastLimitCallbackMap;
     static std::atomic<uint32_t>                sBinderProxyCount;
     static std::atomic<uint32_t>                sBinderProxyCountWarned;
+    static binder_proxy_warning_callback        sWarningCallback;
+    static uint32_t                             sBinderProxyCountWarningWatermark;
 };
 
 } // namespace android
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 0ee96e7317..2cea14f631 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -115,6 +115,7 @@ enum BinderLibTestTranscationCode {
     BINDER_LIB_TEST_GET_SCHEDULING_POLICY,
     BINDER_LIB_TEST_NOP_TRANSACTION_WAIT,
     BINDER_LIB_TEST_GETPID,
+    BINDER_LIB_TEST_GETUID,
     BINDER_LIB_TEST_ECHO_VECTOR,
     BINDER_LIB_TEST_GET_NON_BLOCKING_FD,
     BINDER_LIB_TEST_REJECT_OBJECTS,
@@ -1477,6 +1478,86 @@ TEST_F(BinderLibTest, BinderProxyCount) {
     EXPECT_EQ(BpBinder::getBinderProxyCount(), initialCount);
 }
 
+static constexpr int kBpCountHighWatermark = 20;
+static constexpr int kBpCountLowWatermark = 10;
+static constexpr int kBpCountWarningWatermark = 15;
+static constexpr int kInvalidUid = -1;
+
+TEST_F(BinderLibTest, BinderProxyCountCallback) {
+    Parcel data, reply;
+    sp<IBinder> server = addServer();
+    ASSERT_NE(server, nullptr);
+
+    BpBinder::enableCountByUid();
+    EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_GETUID, data, &reply), StatusEq(NO_ERROR));
+    int32_t uid = reply.readInt32();
+    ASSERT_NE(uid, kInvalidUid);
+
+    uint32_t initialCount = BpBinder::getBinderProxyCount();
+    {
+        uint32_t count = initialCount;
+        BpBinder::setBinderProxyCountWatermarks(kBpCountHighWatermark,
+                                                kBpCountLowWatermark,
+                                                kBpCountWarningWatermark);
+        int limitCallbackUid = kInvalidUid;
+        int warningCallbackUid = kInvalidUid;
+        BpBinder::setBinderProxyCountEventCallback([&](int uid) { limitCallbackUid = uid; },
+                                                   [&](int uid) { warningCallbackUid = uid; });
+
+        std::vector<sp<IBinder> > proxies;
+        auto createProxyOnce = [&](int expectedWarningCallbackUid, int expectedLimitCallbackUid) {
+            warningCallbackUid = limitCallbackUid = kInvalidUid;
+            ASSERT_THAT(server->transact(BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION, data, &reply),
+                        StatusEq(NO_ERROR));
+            proxies.push_back(reply.readStrongBinder());
+            EXPECT_EQ(BpBinder::getBinderProxyCount(), ++count);
+            EXPECT_EQ(warningCallbackUid, expectedWarningCallbackUid);
+            EXPECT_EQ(limitCallbackUid, expectedLimitCallbackUid);
+        };
+        auto removeProxyOnce = [&](int expectedWarningCallbackUid, int expectedLimitCallbackUid) {
+            warningCallbackUid = limitCallbackUid = kInvalidUid;
+            proxies.pop_back();
+            EXPECT_EQ(BpBinder::getBinderProxyCount(), --count);
+            EXPECT_EQ(warningCallbackUid, expectedWarningCallbackUid);
+            EXPECT_EQ(limitCallbackUid, expectedLimitCallbackUid);
+        };
+
+        // Test the increment/decrement of the binder proxies.
+        for (int i = 1; i <= kBpCountWarningWatermark; i++) {
+            createProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(uid, kInvalidUid); // Warning callback should have been triggered.
+        for (int i = kBpCountWarningWatermark + 2; i <= kBpCountHighWatermark; i++) {
+            createProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(kInvalidUid, uid); // Limit callback should have been triggered.
+        createProxyOnce(kInvalidUid, kInvalidUid);
+        for (int i = kBpCountHighWatermark + 2; i >= kBpCountHighWatermark; i--) {
+            removeProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(kInvalidUid, kInvalidUid);
+
+        // Go down below the low watermark.
+        for (int i = kBpCountHighWatermark; i >= kBpCountLowWatermark; i--) {
+            removeProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        for (int i = kBpCountLowWatermark; i <= kBpCountWarningWatermark; i++) {
+            createProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(uid, kInvalidUid); // Warning callback should have been triggered.
+        for (int i = kBpCountWarningWatermark + 2; i <= kBpCountHighWatermark; i++) {
+            createProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(kInvalidUid, uid); // Limit callback should have been triggered.
+        createProxyOnce(kInvalidUid, kInvalidUid);
+        for (int i = kBpCountHighWatermark + 2; i >= kBpCountHighWatermark; i--) {
+            removeProxyOnce(kInvalidUid, kInvalidUid);
+        }
+        createProxyOnce(kInvalidUid, kInvalidUid);
+    }
+    EXPECT_EQ(BpBinder::getBinderProxyCount(), initialCount);
+}
+
 class BinderLibRpcTestBase : public BinderLibTest {
 public:
     void SetUp() override {
@@ -1680,6 +1761,9 @@ public:
             case BINDER_LIB_TEST_GETPID:
                 reply->writeInt32(getpid());
                 return NO_ERROR;
+            case BINDER_LIB_TEST_GETUID:
+                reply->writeInt32(getuid());
+                return NO_ERROR;
             case BINDER_LIB_TEST_NOP_TRANSACTION_WAIT:
                 usleep(5000);
                 [[fallthrough]];
diff --git a/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h b/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
index 0a584bfe56..83d0ca745f 100644
--- a/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
@@ -95,14 +95,16 @@ static const std::vector<std::function<void(FuzzedDataProvider*, const sp<BpBind
                  },
                  [](FuzzedDataProvider*, const sp<BpBinder>& bpbinder,
                     const sp<IBinder::DeathRecipient>&) -> void {
-                     binder_proxy_limit_callback cb = binder_proxy_limit_callback();
-                     bpbinder->setLimitCallback(cb);
+                     binder_proxy_limit_callback cbl = binder_proxy_limit_callback();
+                     binder_proxy_warning_callback cbw = binder_proxy_warning_callback();
+                     bpbinder->setBinderProxyCountEventCallback(cbl, cbw);
                  },
                  [](FuzzedDataProvider* fdp, const sp<BpBinder>& bpbinder,
                     const sp<IBinder::DeathRecipient>&) -> void {
                      int high = fdp->ConsumeIntegral<int>();
                      int low = fdp->ConsumeIntegral<int>();
-                     bpbinder->setBinderProxyCountWatermarks(high, low);
+                     int warning = fdp->ConsumeIntegral<int>();
+                     bpbinder->setBinderProxyCountWatermarks(high, low, warning);
                  }};
 
 } // namespace android