Merge changes Id2c1d0dc,I654fcd2c am: ee22bc842b am: dc246cf8e1

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/1537303

Change-Id: Ifee05e1c82cfc6beebf6b7396ff42d651d8c89ae

6 files changed, 63 insertions, 22 deletions

diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index c83c383513..415b44e683 100644
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -197,9 +197,7 @@ public:
 
 // ---------------------------------------------------------------------------
 
-BBinder::BBinder() : mExtras(nullptr), mStability(0)
-{
-}
+BBinder::BBinder() : mExtras(nullptr), mStability(0), mParceled(false) {}
 
 bool BBinder::isBinderAlive() const
 {
@@ -322,6 +320,10 @@ bool BBinder::isRequestingSid()
 
 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 @@ sp<IBinder> BBinder::getExtension() {
 }
 
 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 @@ bool BBinder::isInheritRt() {
 }
 
 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 @@ pid_t BBinder::getDebugPid() {
 }
 
 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 ae61b366fd..7ab3b754b9 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -173,8 +173,8 @@ static void release_object(const sp<ProcessState>& proc,
 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 Parcel::finishUnflattenBinder(
     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 @@ static constexpr inline int schedPolicyMask(int policy, int priority) {
     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 @@ status_t Parcel::flattenBinder(const sp<IBinder>& binder)
     }
 
     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 3a5575fec5..00b69d527a 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -33,7 +33,6 @@ constexpr uint8_t kBinderWireFormatVersion = 1;
 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 754f87cd7e..d162dda16c 100644
--- a/libs/binder/include/binder/Binder.h
+++ b/libs/binder/include/binder/Binder.h
@@ -94,6 +94,13 @@ public:
 
     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 @@ private:
     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 6bc607f298..629b565985 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -136,14 +136,15 @@ private:
         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 @@ private:
         // 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 3289b5f765..12b54c2d1b 100644
--- a/libs/binder/tests/binderLibTest.cpp
+++ b/libs/binder/tests/binderLibTest.cpp
@@ -443,6 +443,14 @@ class TestDeathRecipient : public IBinder::DeathRecipient, public BinderLibTestE
         };
 };
 
+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),