Skip to content

Commit 54fa6c7

Browse files
author
Steven Moreland
committed
RPC Binder: increase transaction size
One of the RPC Binder usecases requires a larger transaction limit. This change increases the limit, and it unifies the 'too large transaction' logging from regular and RPC binder. This hopefully makes it more clear why there is a limit there, we want to keep code compatible between the two transports. A test is added to show the current behavior. When a transaction which is sent is too large, the server closes the session. This is probably the correct behavior for too large replies, but for too large transactions, the client could handle these errors. b/392717039 is filed to investigate inconsistencies raised in the test more deeply. Fixes: 392575419 Test: atest binderRpcTest --test-filter="*LargeVector*" Change-Id: I2eeb08818c10371c7f77a35abee7d4e46bb63d72
1 parent a5f7d9f commit 54fa6c7

12 files changed

Lines changed: 108 additions & 14 deletions

File tree

libs/binder/Binder.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
#endif
3939

4040
#include "BuildFlags.h"
41+
#include "Constants.h"
4142
#include "OS.h"
4243
#include "RpcState.h"
4344

@@ -70,8 +71,6 @@ constexpr bool kEnableRecording = true;
7071
constexpr bool kEnableRecording = false;
7172
#endif
7273

73-
// Log any reply transactions for which the data exceeds this size
74-
#define LOG_REPLIES_OVER_SIZE (300 * 1024)
7574
// ---------------------------------------------------------------------------
7675

7776
IBinder::IBinder()
@@ -412,7 +411,7 @@ status_t BBinder::transact(
412411
// In case this is being transacted on in the same process.
413412
if (reply != nullptr) {
414413
reply->setDataPosition(0);
415-
if (reply->dataSize() > LOG_REPLIES_OVER_SIZE) {
414+
if (reply->dataSize() > binder::kLogTransactionsOverBytes) {
416415
ALOGW("Large reply transaction of %zu bytes, interface descriptor %s, code %d",
417416
reply->dataSize(), String8(getInterfaceDescriptor()).c_str(), code);
418417
}

libs/binder/BpBinder.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
#include <stdio.h>
2929

3030
#include "BuildFlags.h"
31+
#include "Constants.h"
3132
#include "file.h"
3233

3334
//#undef ALOGV
@@ -63,9 +64,6 @@ std::atomic<uint32_t> BpBinder::sBinderProxyCountWarned(0);
6364

6465
static constexpr uint32_t kBinderProxyCountWarnInterval = 5000;
6566

66-
// Log any transactions for which the data exceeds this size
67-
#define LOG_TRANSACTIONS_OVER_SIZE (300 * 1024)
68-
6967
enum {
7068
LIMIT_REACHED_MASK = 0x80000000, // A flag denoting that the limit has been reached
7169
WARNING_REACHED_MASK = 0x40000000, // A flag denoting that the warning has been reached
@@ -403,9 +401,11 @@ status_t BpBinder::transact(
403401

404402
status = IPCThreadState::self()->transact(binderHandle(), code, data, reply, flags);
405403
}
406-
if (data.dataSize() > LOG_TRANSACTIONS_OVER_SIZE) {
404+
405+
if (data.dataSize() > binder::kLogTransactionsOverBytes) {
407406
RpcMutexUniqueLock _l(mLock);
408-
ALOGW("Large outgoing transaction of %zu bytes, interface descriptor %s, code %d",
407+
ALOGW("Large outgoing transaction of %zu bytes, interface descriptor %s, code %d was "
408+
"sent",
409409
data.dataSize(), String8(mDescriptorCache).c_str(), code);
410410
}
411411

libs/binder/Constants.h

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/*
2+
* Copyright (C) 2025 The Android Open Source Project
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#pragma once
18+
19+
namespace android::binder {
20+
21+
/**
22+
* See also BINDER_VM_SIZE. In kernel binder, the sum of all transactions must be allocated in this
23+
* space. Large transactions are very error prone. In general, we should work to reduce this limit.
24+
* The same limit is used in RPC binder for consistency.
25+
*/
26+
constexpr size_t kLogTransactionsOverBytes = 300 * 1024;
27+
28+
/**
29+
* See b/392575419 - this limit is chosen for a specific usecase, because RPC binder does not have
30+
* support for shared memory in the Android Baklava timeframe. This was 100 KB during and before
31+
* Android V.
32+
*
33+
* Keeping this low helps preserve overall system performance. Transactions of this size are far too
34+
* expensive to make multiple copies over binder or sockets, and they should be avoided if at all
35+
* possible and transition to shared memory.
36+
*/
37+
constexpr size_t kRpcTransactionLimitBytes = 600 * 1024;
38+
39+
} // namespace android::binder

libs/binder/RpcState.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <binder/IPCThreadState.h>
2424
#include <binder/RpcServer.h>
2525

26+
#include "Constants.h"
2627
#include "Debug.h"
2728
#include "RpcWireFormat.h"
2829
#include "Utils.h"
@@ -337,6 +338,8 @@ std::string RpcState::BinderNode::toString() const {
337338
}
338339

339340
RpcState::CommandData::CommandData(size_t size) : mSize(size) {
341+
if (size == 0) return;
342+
340343
// The maximum size for regular binder is 1MB for all concurrent
341344
// transactions. A very small proportion of transactions are even
342345
// larger than a page, but we need to avoid allocating too much
@@ -348,11 +351,11 @@ RpcState::CommandData::CommandData(size_t size) : mSize(size) {
348351
// transaction (in some cases, additional fixed size amounts are added),
349352
// though for rough consistency, we should avoid cases where this data type
350353
// is used for multiple dynamic allocations for a single transaction.
351-
constexpr size_t kMaxTransactionAllocation = 100 * 1000;
352-
if (size == 0) return;
353-
if (size > kMaxTransactionAllocation) {
354-
ALOGW("Transaction requested too much data allocation %zu", size);
354+
if (size > binder::kRpcTransactionLimitBytes) {
355+
ALOGE("Transaction requested too much data allocation: %zu bytes, failing.", size);
355356
return;
357+
} else if (size > binder::kLogTransactionsOverBytes) {
358+
ALOGW("Transaction too large: inefficient and in danger of breaking: %zu bytes.", size);
356359
}
357360
mData.reset(new (std::nothrow) uint8_t[size]);
358361
}

libs/binder/tests/IBinderRpcTest.aidl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ interface IBinderRpcTest {
3434
void holdBinder(@nullable IBinder binder);
3535
@nullable IBinder getHeldBinder();
3636

37+
byte[] repeatBytes(in byte[] bytes);
38+
3739
// Idea is client creates its own instance of IBinderRpcTest and calls this,
3840
// and the server calls 'binder' with (calls - 1) passing itself as 'binder',
3941
// going back and forth until calls = 0

libs/binder/tests/binderRpcTest.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,35 @@ TEST_P(BinderRpc, OnewayCallExhaustion) {
711711
proc.proc->sessions.erase(proc.proc->sessions.begin() + 1);
712712
}
713713

714+
// TODO(b/392717039): can we move this to universal tests?
715+
TEST_P(BinderRpc, SendTooLargeVector) {
716+
if (GetParam().singleThreaded) {
717+
GTEST_SKIP() << "Requires multi-threaded server to test one of the sessions crashing.";
718+
}
719+
720+
auto proc = createRpcTestSocketServerProcess({.numSessions = 2});
721+
722+
// need a working transaction
723+
EXPECT_EQ(OK, proc.rootBinder->pingBinder());
724+
725+
// see libbinder internal Constants.h
726+
const size_t kTooLargeSize = 650 * 1024;
727+
const std::vector<uint8_t> kTestValue(kTooLargeSize / sizeof(uint8_t), 42);
728+
729+
// TODO(b/392717039): Telling a server to allocate too much data currently causes the session to
730+
// close since RpcServer treats any transaction error as a failure. We likely want to change
731+
// this behavior to be a soft failure, since it isn't hard to keep track of this state.
732+
sp<IBinderRpcTest> rootIface2 = interface_cast<IBinderRpcTest>(proc.proc->sessions.at(1).root);
733+
std::vector<uint8_t> result;
734+
status_t res = rootIface2->repeatBytes(kTestValue, &result).transactionError();
735+
736+
// TODO(b/392717039): consistent error results always
737+
EXPECT_TRUE(res == -ECONNRESET || res == DEAD_OBJECT) << statusToString(res);
738+
739+
// died, so remove it for checks in destructor of proc
740+
proc.proc->sessions.erase(proc.proc->sessions.begin() + 1);
741+
}
742+
714743
TEST_P(BinderRpc, SessionWithIncomingThreadpoolDoesntLeak) {
715744
if (clientOrServerSingleThreaded()) {
716745
GTEST_SKIP() << "This test requires multiple threads";

libs/binder/tests/binderRpcTestCommon.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,10 @@ class MyBinderRpcTestBase : public BnBinderRpcTest {
348348
*out = binder;
349349
return Status::ok();
350350
}
351+
Status repeatBytes(const std::vector<uint8_t>& bytes, std::vector<uint8_t>* out) override {
352+
*out = bytes;
353+
return Status::ok();
354+
}
351355
static sp<IBinder> mHeldBinder;
352356
Status holdBinder(const sp<IBinder>& binder) override {
353357
mHeldBinder = binder;

libs/binder/tests/binderRpcUniversalTests.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,18 @@ TEST_P(BinderRpc, RepeatBinder) {
209209
EXPECT_EQ(0, MyBinderRpcSession::gNum);
210210
}
211211

212+
TEST_P(BinderRpc, SendLargeVector) {
213+
auto proc = createRpcTestSocketServerProcess({});
214+
215+
// see libbinder internal Constants.h
216+
const size_t kLargeSize = 550 * 1024;
217+
const std::vector<uint8_t> kTestValue(kLargeSize / sizeof(uint8_t), 42);
218+
219+
std::vector<uint8_t> result;
220+
EXPECT_OK(proc.rootIface->repeatBytes(kTestValue, &result));
221+
EXPECT_EQ(result, kTestValue);
222+
}
223+
212224
TEST_P(BinderRpc, RepeatTheirBinder) {
213225
auto proc = createRpcTestSocketServerProcess({});
214226

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"uuid": "9dbe9fb8-60fd-4bdd-af86-03e95d7ad78b",
33
"app_name": "binderRpcTest",
4-
"min_heap": 262144,
4+
"min_heap": 4194304,
55
"min_stack": 20480
66
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"uuid": "87e424e5-69d7-4bbd-8b7c-7e24812cbc94",
33
"app_name": "binderRpcTestService",
4-
"min_heap": 65536,
4+
"min_heap": 4194304,
55
"min_stack": 20480,
66
"mgmt_flags": {
77
"restart_on_exit": true,

0 commit comments

Comments
 (0)