Merge "RPC Binder: increase transaction size" into main

Author: Treehugger Robot

libs/binder/Binder.cpp

@@ -38,6 +38,7 @@
 #endif
 
 #include "BuildFlags.h"
+#include "Constants.h"
 #include "OS.h"
 #include "RpcState.h"
 
@@ -70,8 +71,6 @@ constexpr bool kEnableRecording = true;
 constexpr bool kEnableRecording = false;
 #endif
 
-// Log any reply transactions for which the data exceeds this size
-#define LOG_REPLIES_OVER_SIZE (300 * 1024)
 // ---------------------------------------------------------------------------
 
 IBinder::IBinder()
@@ -412,7 +411,7 @@ status_t BBinder::transact(
     // In case this is being transacted on in the same process.
     if (reply != nullptr) {
         reply->setDataPosition(0);
-        if (reply->dataSize() > LOG_REPLIES_OVER_SIZE) {
+        if (reply->dataSize() > binder::kLogTransactionsOverBytes) {
             ALOGW("Large reply transaction of %zu bytes, interface descriptor %s, code %d",
                   reply->dataSize(), String8(getInterfaceDescriptor()).c_str(), code);
         }

libs/binder/BpBinder.cpp

@@ -28,6 +28,7 @@
 #include <stdio.h>
 
 #include "BuildFlags.h"
+#include "Constants.h"
 #include "file.h"
 
 //#undef ALOGV
@@ -63,9 +64,6 @@ std::atomic<uint32_t> BpBinder::sBinderProxyCountWarned(0);
 
 static constexpr uint32_t kBinderProxyCountWarnInterval = 5000;
 
-// Log any transactions for which the data exceeds this size
-#define LOG_TRANSACTIONS_OVER_SIZE (300 * 1024)
-
 enum {
     LIMIT_REACHED_MASK = 0x80000000,        // A flag denoting that the limit has been reached
     WARNING_REACHED_MASK = 0x40000000,      // A flag denoting that the warning has been reached
@@ -403,9 +401,11 @@ status_t BpBinder::transact(
 
             status = IPCThreadState::self()->transact(binderHandle(), code, data, reply, flags);
         }
-        if (data.dataSize() > LOG_TRANSACTIONS_OVER_SIZE) {
+
+        if (data.dataSize() > binder::kLogTransactionsOverBytes) {
             RpcMutexUniqueLock _l(mLock);
-            ALOGW("Large outgoing transaction of %zu bytes, interface descriptor %s, code %d",
+            ALOGW("Large outgoing transaction of %zu bytes, interface descriptor %s, code %d was "
+                  "sent",
                   data.dataSize(), String8(mDescriptorCache).c_str(), code);
         }
 

libs/binder/Constants.h

@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2025 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#pragma once
+
+namespace android::binder {
+
+/**
+ * See also BINDER_VM_SIZE. In kernel binder, the sum of all transactions must be allocated in this
+ * space. Large transactions are very error prone. In general, we should work to reduce this limit.
+ * The same limit is used in RPC binder for consistency.
+ */
+constexpr size_t kLogTransactionsOverBytes = 300 * 1024;
+
+/**
+ * See b/392575419 - this limit is chosen for a specific usecase, because RPC binder does not have
+ * support for shared memory in the Android Baklava timeframe. This was 100 KB during and before
+ * Android V.
+ *
+ * Keeping this low helps preserve overall system performance. Transactions of this size are far too
+ * expensive to make multiple copies over binder or sockets, and they should be avoided if at all
+ * possible and transition to shared memory.
+ */
+constexpr size_t kRpcTransactionLimitBytes = 600 * 1024;
+
+} // namespace android::binder

libs/binder/RpcState.cpp

@@ -23,6 +23,7 @@
 #include <binder/IPCThreadState.h>
 #include <binder/RpcServer.h>
 
+#include "Constants.h"
 #include "Debug.h"
 #include "RpcWireFormat.h"
 #include "Utils.h"
@@ -337,6 +338,8 @@ std::string RpcState::BinderNode::toString() const {
 }
 
 RpcState::CommandData::CommandData(size_t size) : mSize(size) {
+    if (size == 0) return;
+
     // The maximum size for regular binder is 1MB for all concurrent
     // transactions. A very small proportion of transactions are even
     // larger than a page, but we need to avoid allocating too much
@@ -348,11 +351,11 @@ RpcState::CommandData::CommandData(size_t size) : mSize(size) {
     // transaction (in some cases, additional fixed size amounts are added),
     // though for rough consistency, we should avoid cases where this data type
     // is used for multiple dynamic allocations for a single transaction.
-    constexpr size_t kMaxTransactionAllocation = 100 * 1000;
-    if (size == 0) return;
-    if (size > kMaxTransactionAllocation) {
-        ALOGW("Transaction requested too much data allocation %zu", size);
+    if (size > binder::kRpcTransactionLimitBytes) {
+        ALOGE("Transaction requested too much data allocation: %zu bytes, failing.", size);
         return;
+    } else if (size > binder::kLogTransactionsOverBytes) {
+        ALOGW("Transaction too large: inefficient and in danger of breaking: %zu bytes.", size);
     }
     mData.reset(new (std::nothrow) uint8_t[size]);
 }

libs/binder/tests/IBinderRpcTest.aidl

@@ -34,6 +34,8 @@ interface IBinderRpcTest {
     void holdBinder(@nullable IBinder binder);
     @nullable IBinder getHeldBinder();
 
+    byte[] repeatBytes(in byte[] bytes);
+
     // Idea is client creates its own instance of IBinderRpcTest and calls this,
     // and the server calls 'binder' with (calls - 1) passing itself as 'binder',
     // going back and forth until calls = 0

libs/binder/tests/binderRpcTest.cpp

@@ -711,6 +711,35 @@ TEST_P(BinderRpc, OnewayCallExhaustion) {
     proc.proc->sessions.erase(proc.proc->sessions.begin() + 1);
 }
 
+// TODO(b/392717039): can we move this to universal tests?
+TEST_P(BinderRpc, SendTooLargeVector) {
+    if (GetParam().singleThreaded) {
+        GTEST_SKIP() << "Requires multi-threaded server to test one of the sessions crashing.";
+    }
+
+    auto proc = createRpcTestSocketServerProcess({.numSessions = 2});
+
+    // need a working transaction
+    EXPECT_EQ(OK, proc.rootBinder->pingBinder());
+
+    // see libbinder internal Constants.h
+    const size_t kTooLargeSize = 650 * 1024;
+    const std::vector<uint8_t> kTestValue(kTooLargeSize / sizeof(uint8_t), 42);
+
+    // TODO(b/392717039): Telling a server to allocate too much data currently causes the session to
+    // close since RpcServer treats any transaction error as a failure. We likely want to change
+    // this behavior to be a soft failure, since it isn't hard to keep track of this state.
+    sp<IBinderRpcTest> rootIface2 = interface_cast<IBinderRpcTest>(proc.proc->sessions.at(1).root);
+    std::vector<uint8_t> result;
+    status_t res = rootIface2->repeatBytes(kTestValue, &result).transactionError();
+
+    // TODO(b/392717039): consistent error results always
+    EXPECT_TRUE(res == -ECONNRESET || res == DEAD_OBJECT) << statusToString(res);
+
+    // died, so remove it for checks in destructor of proc
+    proc.proc->sessions.erase(proc.proc->sessions.begin() + 1);
+}
+
 TEST_P(BinderRpc, SessionWithIncomingThreadpoolDoesntLeak) {
     if (clientOrServerSingleThreaded()) {
         GTEST_SKIP() << "This test requires multiple threads";

libs/binder/tests/binderRpcTestCommon.h

@@ -348,6 +348,10 @@ class MyBinderRpcTestBase : public BnBinderRpcTest {
         *out = binder;
         return Status::ok();
     }
+    Status repeatBytes(const std::vector<uint8_t>& bytes, std::vector<uint8_t>* out) override {
+        *out = bytes;
+        return Status::ok();
+    }
     static sp<IBinder> mHeldBinder;
     Status holdBinder(const sp<IBinder>& binder) override {
         mHeldBinder = binder;

libs/binder/tests/binderRpcUniversalTests.cpp

@@ -209,6 +209,18 @@ TEST_P(BinderRpc, RepeatBinder) {
     EXPECT_EQ(0, MyBinderRpcSession::gNum);
 }
 
+TEST_P(BinderRpc, SendLargeVector) {
+    auto proc = createRpcTestSocketServerProcess({});
+
+    // see libbinder internal Constants.h
+    const size_t kLargeSize = 550 * 1024;
+    const std::vector<uint8_t> kTestValue(kLargeSize / sizeof(uint8_t), 42);
+
+    std::vector<uint8_t> result;
+    EXPECT_OK(proc.rootIface->repeatBytes(kTestValue, &result));
+    EXPECT_EQ(result, kTestValue);
+}
+
 TEST_P(BinderRpc, RepeatTheirBinder) {
     auto proc = createRpcTestSocketServerProcess({});
 

libs/binder/trusty/binderRpcTest/manifest.json

@@ -1,6 +1,6 @@
 {
     "uuid": "9dbe9fb8-60fd-4bdd-af86-03e95d7ad78b",
     "app_name": "binderRpcTest",
-    "min_heap": 262144,
+    "min_heap": 4194304,
     "min_stack": 20480
 }

libs/binder/trusty/binderRpcTest/service/manifest.json

@@ -1,7 +1,7 @@
 {
     "uuid": "87e424e5-69d7-4bbd-8b7c-7e24812cbc94",
     "app_name": "binderRpcTestService",
-    "min_heap": 65536,
+    "min_heap": 4194304,
     "min_stack": 20480,
     "mgmt_flags": {
         "restart_on_exit": true,