libbinder: Add binder already sent checks
These operations should only be done before the binder object
is sent out to another process:
- setRequestingSid
- setMinSchedulerPolicy
- setInheritRt
- setExtension
Add log and abort if these are attempted after the binder object
has been sent already.
Bug: 166282674
Test: binderParcelTest
Change-Id: Id2c1d0dc783cad75754a06a3047cf6c7bf704c63
diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index c83c383..415b44e 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -197,9 +197,7 @@
// ---------------------------------------------------------------------------
-BBinder::BBinder() : mExtras(nullptr), mStability(0)
-{
-}
+BBinder::BBinder() : mExtras(nullptr), mStability(0), mParceled(false) {}
bool BBinder::isBinderAlive() const
{
@@ -322,6 +320,10 @@
void BBinder::setRequestingSid(bool requestingSid)
{
+ ALOGW_IF(mParceled,
+ "setRequestingSid() should not be called after a binder object "
+ "is parceled/sent to another process");
+
Extras* e = mExtras.load(std::memory_order_acquire);
if (!e) {
@@ -344,6 +346,10 @@
}
void BBinder::setMinSchedulerPolicy(int policy, int priority) {
+ ALOGW_IF(mParceled,
+ "setMinSchedulerPolicy() should not be called after a binder object "
+ "is parceled/sent to another process");
+
switch (policy) {
case SCHED_NORMAL:
LOG_ALWAYS_FATAL_IF(priority < -20 || priority > 19, "Invalid priority for SCHED_NORMAL: %d", priority);
@@ -391,6 +397,10 @@
}
void BBinder::setInheritRt(bool inheritRt) {
+ ALOGW_IF(mParceled,
+ "setInheritRt() should not be called after a binder object "
+ "is parceled/sent to another process");
+
Extras* e = mExtras.load(std::memory_order_acquire);
if (!e) {
@@ -410,10 +420,22 @@
}
void BBinder::setExtension(const sp<IBinder>& extension) {
+ ALOGW_IF(mParceled,
+ "setExtension() should not be called after a binder object "
+ "is parceled/sent to another process");
+
Extras* e = getOrCreateExtras();
e->mExtension = extension;
}
+bool BBinder::wasParceled() {
+ return mParceled;
+}
+
+void BBinder::setParceled() {
+ mParceled = true;
+}
+
status_t BBinder::setRpcClientDebug(const Parcel& data) {
if constexpr (!kEnableRpcDevServers) {
ALOGW("%s: disallowed because RPC is not enabled", __PRETTY_FUNCTION__);
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 10188fe..232a70c 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -196,8 +196,11 @@
return (priority & FLAT_BINDER_FLAG_PRIORITY_MASK) | ((policy & 3) << FLAT_BINDER_FLAG_SCHED_POLICY_SHIFT);
}
-status_t Parcel::flattenBinder(const sp<IBinder>& binder)
-{
+status_t Parcel::flattenBinder(const sp<IBinder>& binder) {
+ BBinder* local = nullptr;
+ if (binder) local = binder->localBinder();
+ if (local) local->setParceled();
+
if (isForRpc()) {
if (binder) {
status_t status = writeInt32(1); // non-null
@@ -223,7 +226,6 @@
}
if (binder != nullptr) {
- BBinder *local = binder->localBinder();
if (!local) {
BpBinder *proxy = binder->remoteBinder();
if (proxy == nullptr) {
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 27db32c..d162dda 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -94,6 +94,13 @@
pid_t getDebugPid();
+ // Whether this binder has been sent to another process.
+ bool wasParceled();
+ // Consider this binder as parceled (setup/init-related calls should no
+ // longer by called. This is automatically set by when this binder is sent
+ // to another process.
+ void setParceled();
+
[[nodiscard]] status_t setRpcClientDebug(android::base::unique_fd clientFd,
uint32_t maxRpcThreads);
@@ -120,7 +127,8 @@
friend ::android::internal::Stability;
int16_t mStability;
- int16_t mReserved0;
+ bool mParceled;
+ uint8_t mReserved0;
#ifdef __LP64__
int32_t mReserved1;
diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp
index 3289b5f..12b54c2 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -443,6 +443,14 @@
};
};
+TEST_F(BinderLibTest, WasParceled) {
+ auto binder = sp<BBinder>::make();
+ EXPECT_FALSE(binder->wasParceled());
+ Parcel data;
+ data.writeStrongBinder(binder);
+ EXPECT_TRUE(binder->wasParceled());
+}
+
TEST_F(BinderLibTest, NopTransaction) {
Parcel data, reply;
EXPECT_THAT(m_server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data, &reply),