Add a new callback for the excessive binder proxy count
The callback is served as a warning where there are too many
binder proxies from a certain UID. The recipient of the callback
would then have a chance to log this event and/or trigger a dump,
before things getting out-of-control and trigger the high watermark
callback, where the process could be killed.
This callback signal could also be used as a hint to the GC in
the upper layer, which could effectively release the references to
remote binder stubs.
(cherry-picked from ag/25082740)
Bug: 298263955
Test: binder_bpBinderFuzz
Test: atest binderLibTest
Change-Id: I5d3659a9430c7404f7cfdfe4732980f5b1b0fa65
Merged-In: I5d3659a9430c7404f7cfdfe4732980f5b1b0fa65
diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 42dd691..54457fc 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -44,6 +44,7 @@
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 @@
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 @@
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 @@
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 @@
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::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 89a4d27..9f03907 100644
--- a/libs/binder/include/binder/BpBinder.h
+++ b/libs/binder/include/binder/BpBinder.h
@@ -35,7 +35,8 @@
}
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 @@
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 @@
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 0ee96e7..2cea14f 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -115,6 +115,7 @@
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 @@
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 @@
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 0a584bf..83d0ca7 100644
--- a/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
+++ b/libs/binder/tests/unit_fuzzers/BpBinderFuzzFunctions.h
@@ -95,14 +95,16 @@
},
[](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