Loading libs/binder/Constants.h +0 −3 Original line number Diff line number Diff line Loading @@ -36,7 +36,4 @@ constexpr size_t kLogTransactionsOverBytes = 300 * 1024; */ constexpr size_t kRpcTransactionLimitBytes = 600 * 1024; // TODO(b/424526253): remove high limit again constexpr size_t kRpcTransactionTemporaryLimitBytes = 20 * 1024 * 1024; } // namespace android::binder libs/binder/RpcState.cpp +42 −11 Original line number Diff line number Diff line Loading @@ -378,14 +378,9 @@ 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. if (size > binder::kRpcTransactionTemporaryLimitBytes) { ALOGE("Transaction requested WAY WAY WAY too much data allocation: %zu bytes, failing.", size); if (size > binder::kRpcTransactionLimitBytes) { ALOGE("Transaction requested too much data allocation: %zu bytes, failing.", size); return; } else if (size > binder::kRpcTransactionLimitBytes) { ALOGE("Transaction requested too much data allocation: %zu bytes, this will become a " "failure!!!", size); } else if (size > binder::kLogTransactionsOverBytes) { ALOGW("Transaction too large: inefficient and in danger of breaking: %zu bytes.", size); } Loading Loading @@ -624,6 +619,17 @@ status_t RpcState::transactInternal(const sp<RpcSession::RpcConnection>& connect address = RPC_SPECIAL_TRANSACTION_ADDRESS; } scope_guard onBinderLeavingGuard = make_scope_guard([&]() { if (!maybeBinder) return; if (status_t status = cancelBinderLeaving(session, address); status != OK) { ALOGE("Failed to fix reference count when canceling transaction %s", statusToString(status).c_str()); (void)session->shutdownAndWait(false); // we'd like to return DEAD_OBJECT from the outer function here, but // rpcSend will immediately fail } }); if (!(flags & IBinder::FLAG_ONEWAY)) { LOG_ALWAYS_FATAL_IF(reply == nullptr, "Reply parcel must be used for synchronous transaction."); Loading Loading @@ -661,10 +667,17 @@ status_t RpcState::transactInternal(const sp<RpcSession::RpcConnection>& connect &bodySize), "Too much data %zu", data.dataSize()); if (bodySize >= binder::kRpcTransactionLimitBytes - sizeof(RpcWireHeader)) { // fail here rather than having client allocate a huge amount of data ALOGE("Transaction for code %d too large: %" PRIu32 " body size bytes.", code, bodySize); return FAILED_TRANSACTION; } // At this point, all errors imply a protocol break and the transaction must be shut // down to avoid leaks. Any errors before this point while constructing this Parcel // can be ignored. data.rpcSend(); onBinderLeavingGuard.release(); RpcWireHeader command{ .command = RPC_COMMAND_TRANSACT, Loading Loading @@ -1297,11 +1310,29 @@ processTransactInternalTailCall: rpcFields->mObjectPositions.size()}; uint32_t bodySize; while (true) { LOG_ALWAYS_FATAL_IF(__builtin_add_overflow(rpcReplyWireSize, reply.dataSize(), &bodySize) || __builtin_add_overflow(objectTableSpan.byteSize(), bodySize, &bodySize), "Too much data for reply %zu", reply.dataSize()); if (bodySize < binder::kRpcTransactionLimitBytes - sizeof(RpcWireHeader)) { break; } // Fail here, rather than requesting the client to allocate a huge amount. // See waitForReply. There we have a separate rpcRec for the header (at the // time of writing). However, it's better to impose the limit over the // entire packet, as +/- a few bytes doesn't matter, this would work even // if those are combined, and it errs on making the packet here slightly // smaller. ALOGE("Reply transaction for code %d too large: %" PRIu32 " body size bytes.", transaction->code, bodySize); reply.setDataSize(0); objectTableSpan.clear(); replyStatus = FAILED_TRANSACTION; // match kernel binder } // Ownership of objects in Parcel is considered to be the receiver at this point. All errors // above should be sent through "replyStatus" anyway. However, for other Parcel which are // constructed elsewhere, this won't get called, so their resources will get cleaned up. Loading libs/binder/Utils.h +5 −0 Original line number Diff line number Diff line Loading @@ -106,6 +106,11 @@ struct Span { iovec toIovec() { return {const_cast<std::remove_const_t<T>*>(data), byteSize()}; } void clear() { data = nullptr; size = 0; } // Truncates `this` to a length of `offset` and returns a span with the // remainder. // Loading libs/binder/tests/binderRpcTest.cpp +8 −13 Original line number Diff line number Diff line Loading @@ -713,33 +713,28 @@ TEST_P(BinderRpc, OnewayCallExhaustion) { proc.proc->sessions.erase(proc.proc->sessions.begin() + 1); } // TODO(b/392717039): can we move this to universal tests? // TODO(b/424526253): restore limit down // TODO(b/392717039): move 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}); auto proc = createRpcTestSocketServerProcess({}); // need a working transaction // works before EXPECT_EQ(OK, proc.rootBinder->pingBinder()); // see libbinder internal Constants.h const size_t kTooLargeSize = 25 * 1024 * 1024; 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(); status_t res = proc.rootIface->repeatBytes(kTestValue, &result).transactionError(); EXPECT_EQ(res, DEAD_OBJECT) << statusToString(res); EXPECT_EQ(res, FAILED_TRANSACTION) << statusToString(res); // died, so remove it for checks in destructor of proc proc.proc->sessions.erase(proc.proc->sessions.begin() + 1); // works after EXPECT_EQ(OK, proc.rootBinder->pingBinder()); } TEST_P(BinderRpc, SessionWithIncomingThreadpoolDoesntLeak) { Loading libs/binder/tests/binderRpcUniversalTests.cpp +1 −1 Original line number Diff line number Diff line Loading @@ -249,7 +249,7 @@ TEST_P(BinderRpc, SendLargeVector) { // see libbinder internal Constants.h // We use a smaller size for TIPC because a 15MB test is too slow and times out size_t kLargeSize = socketType() == SocketType::TIPC ? 128 * 1024 : 1 * 1024 * 1024; size_t kLargeSize = socketType() == SocketType::TIPC ? 128 * 1024 : 550 * 1024; const std::vector<uint8_t> kTestValue(kLargeSize / sizeof(uint8_t), 42); std::vector<uint8_t> result; Loading Loading
libs/binder/Constants.h +0 −3 Original line number Diff line number Diff line Loading @@ -36,7 +36,4 @@ constexpr size_t kLogTransactionsOverBytes = 300 * 1024; */ constexpr size_t kRpcTransactionLimitBytes = 600 * 1024; // TODO(b/424526253): remove high limit again constexpr size_t kRpcTransactionTemporaryLimitBytes = 20 * 1024 * 1024; } // namespace android::binder
libs/binder/RpcState.cpp +42 −11 Original line number Diff line number Diff line Loading @@ -378,14 +378,9 @@ 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. if (size > binder::kRpcTransactionTemporaryLimitBytes) { ALOGE("Transaction requested WAY WAY WAY too much data allocation: %zu bytes, failing.", size); if (size > binder::kRpcTransactionLimitBytes) { ALOGE("Transaction requested too much data allocation: %zu bytes, failing.", size); return; } else if (size > binder::kRpcTransactionLimitBytes) { ALOGE("Transaction requested too much data allocation: %zu bytes, this will become a " "failure!!!", size); } else if (size > binder::kLogTransactionsOverBytes) { ALOGW("Transaction too large: inefficient and in danger of breaking: %zu bytes.", size); } Loading Loading @@ -624,6 +619,17 @@ status_t RpcState::transactInternal(const sp<RpcSession::RpcConnection>& connect address = RPC_SPECIAL_TRANSACTION_ADDRESS; } scope_guard onBinderLeavingGuard = make_scope_guard([&]() { if (!maybeBinder) return; if (status_t status = cancelBinderLeaving(session, address); status != OK) { ALOGE("Failed to fix reference count when canceling transaction %s", statusToString(status).c_str()); (void)session->shutdownAndWait(false); // we'd like to return DEAD_OBJECT from the outer function here, but // rpcSend will immediately fail } }); if (!(flags & IBinder::FLAG_ONEWAY)) { LOG_ALWAYS_FATAL_IF(reply == nullptr, "Reply parcel must be used for synchronous transaction."); Loading Loading @@ -661,10 +667,17 @@ status_t RpcState::transactInternal(const sp<RpcSession::RpcConnection>& connect &bodySize), "Too much data %zu", data.dataSize()); if (bodySize >= binder::kRpcTransactionLimitBytes - sizeof(RpcWireHeader)) { // fail here rather than having client allocate a huge amount of data ALOGE("Transaction for code %d too large: %" PRIu32 " body size bytes.", code, bodySize); return FAILED_TRANSACTION; } // At this point, all errors imply a protocol break and the transaction must be shut // down to avoid leaks. Any errors before this point while constructing this Parcel // can be ignored. data.rpcSend(); onBinderLeavingGuard.release(); RpcWireHeader command{ .command = RPC_COMMAND_TRANSACT, Loading Loading @@ -1297,11 +1310,29 @@ processTransactInternalTailCall: rpcFields->mObjectPositions.size()}; uint32_t bodySize; while (true) { LOG_ALWAYS_FATAL_IF(__builtin_add_overflow(rpcReplyWireSize, reply.dataSize(), &bodySize) || __builtin_add_overflow(objectTableSpan.byteSize(), bodySize, &bodySize), "Too much data for reply %zu", reply.dataSize()); if (bodySize < binder::kRpcTransactionLimitBytes - sizeof(RpcWireHeader)) { break; } // Fail here, rather than requesting the client to allocate a huge amount. // See waitForReply. There we have a separate rpcRec for the header (at the // time of writing). However, it's better to impose the limit over the // entire packet, as +/- a few bytes doesn't matter, this would work even // if those are combined, and it errs on making the packet here slightly // smaller. ALOGE("Reply transaction for code %d too large: %" PRIu32 " body size bytes.", transaction->code, bodySize); reply.setDataSize(0); objectTableSpan.clear(); replyStatus = FAILED_TRANSACTION; // match kernel binder } // Ownership of objects in Parcel is considered to be the receiver at this point. All errors // above should be sent through "replyStatus" anyway. However, for other Parcel which are // constructed elsewhere, this won't get called, so their resources will get cleaned up. Loading
libs/binder/Utils.h +5 −0 Original line number Diff line number Diff line Loading @@ -106,6 +106,11 @@ struct Span { iovec toIovec() { return {const_cast<std::remove_const_t<T>*>(data), byteSize()}; } void clear() { data = nullptr; size = 0; } // Truncates `this` to a length of `offset` and returns a span with the // remainder. // Loading
libs/binder/tests/binderRpcTest.cpp +8 −13 Original line number Diff line number Diff line Loading @@ -713,33 +713,28 @@ TEST_P(BinderRpc, OnewayCallExhaustion) { proc.proc->sessions.erase(proc.proc->sessions.begin() + 1); } // TODO(b/392717039): can we move this to universal tests? // TODO(b/424526253): restore limit down // TODO(b/392717039): move 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}); auto proc = createRpcTestSocketServerProcess({}); // need a working transaction // works before EXPECT_EQ(OK, proc.rootBinder->pingBinder()); // see libbinder internal Constants.h const size_t kTooLargeSize = 25 * 1024 * 1024; 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(); status_t res = proc.rootIface->repeatBytes(kTestValue, &result).transactionError(); EXPECT_EQ(res, DEAD_OBJECT) << statusToString(res); EXPECT_EQ(res, FAILED_TRANSACTION) << statusToString(res); // died, so remove it for checks in destructor of proc proc.proc->sessions.erase(proc.proc->sessions.begin() + 1); // works after EXPECT_EQ(OK, proc.rootBinder->pingBinder()); } TEST_P(BinderRpc, SessionWithIncomingThreadpoolDoesntLeak) { Loading
libs/binder/tests/binderRpcUniversalTests.cpp +1 −1 Original line number Diff line number Diff line Loading @@ -249,7 +249,7 @@ TEST_P(BinderRpc, SendLargeVector) { // see libbinder internal Constants.h // We use a smaller size for TIPC because a 15MB test is too slow and times out size_t kLargeSize = socketType() == SocketType::TIPC ? 128 * 1024 : 1 * 1024 * 1024; size_t kLargeSize = socketType() == SocketType::TIPC ? 128 * 1024 : 550 * 1024; const std::vector<uint8_t> kTestValue(kLargeSize / sizeof(uint8_t), 42); std::vector<uint8_t> result; Loading