summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--libs/binder/Parcel.cpp8
-rw-r--r--libs/binder/RpcState.cpp22
-rw-r--r--libs/binder/include/binder/Parcel.h8
-rw-r--r--libs/binder/include/binder/RpcConnection.h1
-rw-r--r--libs/binder/ndk/ibinder.cpp2
-rw-r--r--libs/binder/ndk/parcel_internal.h6
-rw-r--r--libs/binder/tests/binderRpcTest.cpp7
7 files changed, 41 insertions, 13 deletions
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 34a474b252..3f0b0df4f6 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -555,12 +555,17 @@ void Parcel::markSensitive() const
}
void Parcel::markForBinder(const sp<IBinder>& binder) {
+ LOG_ALWAYS_FATAL_IF(mData != nullptr, "format must be set before data is written");
+
if (binder && binder->remoteBinder() && binder->remoteBinder()->isRpcBinder()) {
markForRpc(binder->remoteBinder()->getPrivateAccessorForId().rpcConnection());
}
}
void Parcel::markForRpc(const sp<RpcConnection>& connection) {
+ LOG_ALWAYS_FATAL_IF(mData != nullptr && mOwner == nullptr,
+ "format must be set before data is written OR on IPC data");
+
LOG_ALWAYS_FATAL_IF(connection == nullptr, "markForRpc requires connection");
mConnection = connection;
}
@@ -2100,6 +2105,9 @@ size_t Parcel::ipcObjectsCount() const
void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize,
const binder_size_t* objects, size_t objectsCount, release_func relFunc)
{
+ // this code uses 'mOwner == nullptr' to understand whether it owns memory
+ LOG_ALWAYS_FATAL_IF(relFunc == nullptr, "must provide cleanup function");
+
freeData();
mData = const_cast<uint8_t*>(data);
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index 3b3adca34f..755ff35781 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -312,8 +312,8 @@ status_t RpcState::transact(const base::unique_fd& fd, const RpcAddress& address
return waitForReply(fd, connection, reply);
}
-static void cleanup_data(Parcel* p, const uint8_t* data, size_t dataSize,
- const binder_size_t* objects, size_t objectsCount) {
+static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize,
+ const binder_size_t* objects, size_t objectsCount) {
(void)p;
delete[] const_cast<uint8_t*>(data - offsetof(RpcWireReply, data));
(void)dataSize;
@@ -351,7 +351,7 @@ status_t RpcState::waitForReply(const base::unique_fd& fd, const sp<RpcConnectio
if (rpcReply->status != OK) return rpcReply->status;
reply->ipcSetDataReference(rpcReply->data, command.bodySize - offsetof(RpcWireReply, data),
- nullptr, 0, cleanup_data);
+ nullptr, 0, cleanup_reply_data);
reply->markForRpc(connection);
@@ -427,6 +427,15 @@ status_t RpcState::processTransact(const base::unique_fd& fd, const sp<RpcConnec
return processTransactInternal(fd, connection, std::move(transactionData));
}
+static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,
+ const binder_size_t* objects, size_t objectsCount) {
+ (void)p;
+ (void)data;
+ (void)dataSize;
+ (void)objects;
+ (void)objectsCount;
+}
+
status_t RpcState::processTransactInternal(const base::unique_fd& fd,
const sp<RpcConnection>& connection,
std::vector<uint8_t>&& transactionData) {
@@ -490,7 +499,12 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd,
}
Parcel data;
- data.setData(transaction->data, transactionData.size() - offsetof(RpcWireTransaction, data));
+ // transaction->data is owned by this function. Parcel borrows this data and
+ // only holds onto it for the duration of this function call. Parcel will be
+ // deleted before the 'transactionData' object.
+ data.ipcSetDataReference(transaction->data,
+ transactionData.size() - offsetof(RpcWireTransaction, data),
+ nullptr /*object*/, 0 /*objectCount*/, do_nothing_to_transact_data);
data.markForRpc(connection);
Parcel reply;
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 957837233b..211790d14c 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -101,10 +101,6 @@ public:
// is for an RPC transaction).
void markForBinder(const sp<IBinder>& binder);
- // Whenever possible, markForBinder should be preferred. This method is
- // called automatically on reply Parcels for RPC transactions.
- void markForRpc(const sp<RpcConnection>& connection);
-
// Whether this Parcel is written for RPC transactions (after calls to
// markForBinder or markForRpc).
bool isForRpc() const;
@@ -540,6 +536,10 @@ private:
const binder_size_t* objects, size_t objectsCount,
release_func relFunc);
+ // Whenever possible, markForBinder should be preferred. This method is
+ // called automatically on reply Parcels for RPC transactions.
+ void markForRpc(const sp<RpcConnection>& connection);
+
status_t finishWrite(size_t len);
void releaseObjects();
void acquireObjects();
diff --git a/libs/binder/include/binder/RpcConnection.h b/libs/binder/include/binder/RpcConnection.h
index a300575454..efa922dbda 100644
--- a/libs/binder/include/binder/RpcConnection.h
+++ b/libs/binder/include/binder/RpcConnection.h
@@ -106,6 +106,7 @@ public:
};
private:
+ friend sp<RpcConnection>;
RpcConnection();
bool addServer(const SocketAddress& address);
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index 0f59de4309..76afd4cc2c 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -598,6 +598,8 @@ binder_status_t AIBinder_prepareTransaction(AIBinder* binder, AParcel** in) {
}
*in = new AParcel(binder);
+ (*in)->get()->markForBinder(binder->getBinder());
+
status_t status = (*in)->get()->writeInterfaceToken(clazz->getInterfaceDescriptor());
binder_status_t ret = PruneStatusT(status);
diff --git a/libs/binder/ndk/parcel_internal.h b/libs/binder/ndk/parcel_internal.h
index aebc7f4539..b4f6358841 100644
--- a/libs/binder/ndk/parcel_internal.h
+++ b/libs/binder/ndk/parcel_internal.h
@@ -29,11 +29,7 @@ struct AParcel {
explicit AParcel(AIBinder* binder) : AParcel(binder, new ::android::Parcel, true /*owns*/) {}
AParcel(AIBinder* binder, ::android::Parcel* parcel, bool owns)
- : mBinder(binder), mParcel(parcel), mOwns(owns) {
- if (binder != nullptr) {
- parcel->markForBinder(binder->getBinder());
- }
- }
+ : mBinder(binder), mParcel(parcel), mOwns(owns) {}
~AParcel() {
if (mOwns) {
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 5f68a25f74..985d086d73 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -44,6 +44,13 @@
namespace android {
+TEST(BinderRpcParcel, EntireParcelFormatted) {
+ Parcel p;
+ p.writeInt32(3);
+
+ EXPECT_DEATH(p.markForBinder(sp<BBinder>::make()), "");
+}
+
using android::binder::Status;
#define EXPECT_OK(status) \
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-11-30 06:27:35 +0000