libbinder: ensure entire Parcel is the same format
Extra check in preparation for versioning work/as a guard against misuse
of this API. Note - these functions are very much like constructors, but
they can't be because Parcel is that errorprone type of object which
resets its internal state (for better or worse).
Bug: 182938972
Test: binderRpcTest
Change-Id: Ibdbe8161db9d0c8fba86f1c691c73aab49b21bc0
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 96d12ca..3f0b0df 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -555,12 +555,17 @@
}
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;
}
diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h
index 9578372..211790d 100644
--- a/libs/binder/include/binder/Parcel.h
+++ b/libs/binder/include/binder/Parcel.h
@@ -101,10 +101,6 @@
// 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 @@
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 a300575..efa922d 100644
--- a/libs/binder/include/binder/RpcConnection.h
+++ b/libs/binder/include/binder/RpcConnection.h
@@ -106,6 +106,7 @@
};
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 0f59de4..76afd4c 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -598,6 +598,8 @@
}
*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 aebc7f4..b4f6358 100644
--- a/libs/binder/ndk/parcel_internal.h
+++ b/libs/binder/ndk/parcel_internal.h
@@ -29,11 +29,7 @@
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 5f68a25..985d086 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) \