Merge changes Id2c1d0dc,I654fcd2c
* changes:
libbinder: Add binder already sent checks
libbinder: +2 bytes in BBinder from stability rep
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 d19b4d8..232a70c 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -173,8 +173,8 @@
status_t Parcel::finishFlattenBinder(const sp<IBinder>& binder)
{
internal::Stability::tryMarkCompilationUnit(binder.get());
- auto category = internal::Stability::getCategory(binder.get());
- return writeInt32(category.repr());
+ int16_t rep = internal::Stability::getCategory(binder.get()).repr();
+ return writeInt32(rep);
}
status_t Parcel::finishUnflattenBinder(
@@ -184,7 +184,8 @@
status_t status = readInt32(&stability);
if (status != OK) return status;
- status = internal::Stability::setRepr(binder.get(), stability, true /*log*/);
+ status = internal::Stability::setRepr(binder.get(), static_cast<int16_t>(stability),
+ true /*log*/);
if (status != OK) return status;
*out = binder;
@@ -195,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
@@ -222,7 +226,6 @@
}
if (binder != nullptr) {
- BBinder *local = binder->localBinder();
if (!local) {
BpBinder *proxy = binder->remoteBinder();
if (proxy == nullptr) {
diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp
index 601ce96..dac3819 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -33,7 +33,6 @@
Stability::Category Stability::Category::currentFromLevel(Level level) {
return {
.version = kBinderWireFormatVersion,
- .reserved = {0},
.level = level,
};
}
diff --git a/libs/binder/include/binder/Binder.h b/libs/binder/include/binder/Binder.h
index 754f87c..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);
@@ -119,10 +126,13 @@
std::atomic<Extras*> mExtras;
friend ::android::internal::Stability;
- union {
- int32_t mStability;
- void* mReserved0;
- };
+ int16_t mStability;
+ bool mParceled;
+ uint8_t mReserved0;
+
+#ifdef __LP64__
+ int32_t mReserved1;
+#endif
};
// ---------------------------------------------------------------------------
diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h
index 6bc607f..629b565 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -136,14 +136,15 @@
VINTF = 0b111111,
};
- // This is the format of stability passed on the wire.
+ // This is the format of stability passed on the wire. It is only 2 bytes
+ // long, but 2 bytes in addition to this are reserved here. The difference
+ // in size is in order to free up space in BBinder, which is fixed by
+ // prebuilts inheriting from it.
struct Category {
- static inline Category fromRepr(int32_t representation) {
+ static inline Category fromRepr(int16_t representation) {
return *reinterpret_cast<Category*>(&representation);
}
- int32_t repr() const {
- return *reinterpret_cast<const int32_t*>(this);
- }
+ int16_t repr() const { return *reinterpret_cast<const int16_t*>(this); }
static inline Category currentFromLevel(Level level);
bool operator== (const Category& o) const {
@@ -161,12 +162,10 @@
// class must write parcels according to the version documented here.
uint8_t version;
- uint8_t reserved[2];
-
// bitmask of Stability::Level
Level level;
};
- static_assert(sizeof(Category) == sizeof(int32_t));
+ static_assert(sizeof(Category) == sizeof(int16_t));
// returns the stability according to how this was built
static Level getLocalLevel();
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),