Merge "libbinder: remove per-binder wire protocol version"

4 files changed, 50 insertions, 119 deletions

diff --git a/libs/binder/BpBinder.cpp b/libs/binder/BpBinder.cpp
index 3099296219..765e21cb38 100644
--- a/libs/binder/BpBinder.cpp
+++ b/libs/binder/BpBinder.cpp
@@ -261,15 +261,15 @@ status_t BpBinder::transact(
         if (code >= FIRST_CALL_TRANSACTION && code <= LAST_CALL_TRANSACTION) {
             using android::internal::Stability;
 
-            auto category = Stability::getCategory(this);
+            int16_t stability = Stability::getRepr(this);
             Stability::Level required = privateVendor ? Stability::VENDOR
                 : Stability::getLocalLevel();
 
-            if (CC_UNLIKELY(!Stability::check(category, required))) {
+            if (CC_UNLIKELY(!Stability::check(stability, required))) {
                 ALOGE("Cannot do a user transaction on a %s binder (%s) in a %s context.",
-                    category.debugString().c_str(),
-                    String8(getInterfaceDescriptor()).c_str(),
-                    Stability::levelString(required).c_str());
+                      Stability::levelString(stability).c_str(),
+                      String8(getInterfaceDescriptor()).c_str(),
+                      Stability::levelString(required).c_str());
                 return BAD_TYPE;
             }
         }
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index ebba375a79..956524ac74 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -173,7 +173,7 @@ static void release_object(const sp<ProcessState>& proc,
 status_t Parcel::finishFlattenBinder(const sp<IBinder>& binder)
 {
     internal::Stability::tryMarkCompilationUnit(binder.get());
-    int16_t rep = internal::Stability::getCategory(binder.get()).repr();
+    int16_t rep = internal::Stability::getRepr(binder.get());
     return writeInt32(rep);
 }
 
diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp
index dac381974e..d25c038df5 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -23,20 +23,6 @@
 namespace android {
 namespace internal {
 
-// the libbinder parcel format is currently unstable
-
-// oldest version which is supported
-constexpr uint8_t kBinderWireFormatOldest = 1;
-// current version
-constexpr uint8_t kBinderWireFormatVersion = 1;
-
-Stability::Category Stability::Category::currentFromLevel(Level level) {
-    return {
-        .version = kBinderWireFormatVersion,
-        .level = level,
-    };
-}
-
 void Stability::forceDowngradeToStability(const sp<IBinder>& binder, Level level) {
     // Downgrading a remote binder would require also copying the version from
     // the binder sent here. In practice though, we don't need to downgrade the
@@ -44,8 +30,7 @@ void Stability::forceDowngradeToStability(const sp<IBinder>& binder, Level level
     // what we can do to it.
     LOG_ALWAYS_FATAL_IF(!binder || !binder->localBinder(), "Can only downgrade local binder");
 
-    auto stability = Category::currentFromLevel(level);
-    status_t result = setRepr(binder.get(), stability.repr(), REPR_LOG | REPR_ALLOW_DOWNGRADE);
+    status_t result = setRepr(binder.get(), level, REPR_LOG | REPR_ALLOW_DOWNGRADE);
     LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
 }
 
@@ -61,41 +46,31 @@ void Stability::forceDowngradeToVendorStability(const sp<IBinder>& binder) {
     forceDowngradeToStability(binder, Level::VENDOR);
 }
 
-std::string Stability::Category::debugString() {
-    return levelString(level) + " wire protocol version "
-        + std::to_string(version);
-}
-
 void Stability::markCompilationUnit(IBinder* binder) {
-    auto stability = Category::currentFromLevel(getLocalLevel());
-    status_t result = setRepr(binder, stability.repr(), REPR_LOG);
+    status_t result = setRepr(binder, getLocalLevel(), REPR_LOG);
     LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
 }
 
 void Stability::markVintf(IBinder* binder) {
-    auto stability = Category::currentFromLevel(Level::VINTF);
-    status_t result = setRepr(binder, stability.repr(), REPR_LOG);
+    status_t result = setRepr(binder, Level::VINTF, REPR_LOG);
     LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
 }
 
 std::string Stability::debugToString(const sp<IBinder>& binder) {
-    auto stability = getCategory(binder.get());
-    return stability.debugString();
+    return levelString(getRepr(binder.get()));
 }
 
 void Stability::markVndk(IBinder* binder) {
-    auto stability = Category::currentFromLevel(Level::VENDOR);
-    status_t result = setRepr(binder, stability.repr(), REPR_LOG);
+    status_t result = setRepr(binder, Level::VENDOR, REPR_LOG);
     LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
 }
 
 bool Stability::requiresVintfDeclaration(const sp<IBinder>& binder) {
-    return check(getCategory(binder.get()), Level::VINTF);
+    return check(getRepr(binder.get()), Level::VINTF);
 }
 
 void Stability::tryMarkCompilationUnit(IBinder* binder) {
-    auto stability = Category::currentFromLevel(getLocalLevel());
-    (void) setRepr(binder, stability.repr(), REPR_NONE);
+    (void)setRepr(binder, getLocalLevel(), REPR_NONE);
 }
 
 Stability::Level Stability::getLocalLevel() {
@@ -117,92 +92,77 @@ Stability::Level Stability::getLocalLevel() {
 #endif
 }
 
-status_t Stability::setRepr(IBinder* binder, int32_t representation, uint32_t flags) {
+status_t Stability::setRepr(IBinder* binder, int32_t setting, uint32_t flags) {
     bool log = flags & REPR_LOG;
     bool allowDowngrade = flags & REPR_ALLOW_DOWNGRADE;
 
-    auto current = getCategory(binder);
-    auto setting = Category::fromRepr(representation);
-
-    // If we have ahold of a binder with a newer declared version, then it
-    // should support older versions, and we will simply write our parcels with
-    // the current wire parcel format.
-    if (setting.version < kBinderWireFormatOldest) {
-        // always log, because this shouldn't happen
-        ALOGE("Cannot accept binder with older binder wire protocol version "
-              "%u. Versions less than %u are unsupported.", setting.version,
-               kBinderWireFormatOldest);
-        return BAD_TYPE;
-    }
+    int16_t current = getRepr(binder);
 
     // null binder is always written w/ 'UNDECLARED' stability
     if (binder == nullptr) {
-        if (setting.level == UNDECLARED) {
+        if (setting == UNDECLARED) {
             return OK;
         } else {
             if (log) {
-                ALOGE("Null binder written with stability %s.",
-                    levelString(setting.level).c_str());
+                ALOGE("Null binder written with stability %s.", levelString(setting).c_str());
             }
             return BAD_TYPE;
         }
     }
 
-    if (!isDeclaredLevel(setting.level)) {
+    if (!isDeclaredLevel(setting)) {
         if (log) {
-            ALOGE("Can only set known stability, not %u.", setting.level);
+            ALOGE("Can only set known stability, not %d.", setting);
         }
         return BAD_TYPE;
     }
+    Level levelSetting = static_cast<Level>(setting);
 
     if (current == setting) return OK;
 
-    bool hasAlreadyBeenSet = current.repr() != 0;
-    bool isAllowedDowngrade = allowDowngrade && check(current, setting.level);
+    bool hasAlreadyBeenSet = current != Level::UNDECLARED;
+    bool isAllowedDowngrade = allowDowngrade && check(current, levelSetting);
     if (hasAlreadyBeenSet && !isAllowedDowngrade) {
         if (log) {
             ALOGE("Interface being set with %s but it is already marked as %s",
-                  setting.debugString().c_str(),
-                  current.debugString().c_str());
+                  levelString(setting).c_str(), levelString(current).c_str());
         }
         return BAD_TYPE;
     }
 
     if (isAllowedDowngrade) {
-        ALOGI("Interface set with %s downgraded to %s stability",
-              current.debugString().c_str(),
-              setting.debugString().c_str());
+        ALOGI("Interface set with %s downgraded to %s stability", levelString(current).c_str(),
+              levelString(setting).c_str());
     }
 
     BBinder* local = binder->localBinder();
     if (local != nullptr) {
-        local->mStability = setting.repr();
+        local->mStability = setting;
     } else {
-        binder->remoteBinder()->mStability = setting.repr();
+        binder->remoteBinder()->mStability = setting;
     }
 
     return OK;
 }
 
-Stability::Category Stability::getCategory(IBinder* binder) {
+int16_t Stability::getRepr(IBinder* binder) {
     if (binder == nullptr) {
-        return Category::currentFromLevel(Level::UNDECLARED);
+        return Level::UNDECLARED;
     }
 
     BBinder* local = binder->localBinder();
     if (local != nullptr) {
-        return Category::fromRepr(local->mStability);
+        return local->mStability;
     }
 
-    return Category::fromRepr(binder->remoteBinder()->mStability);
+    return binder->remoteBinder()->mStability;
 }
 
-bool Stability::check(Category provided, Level required) {
-    bool stable = (provided.level & required) == required;
+bool Stability::check(int16_t provided, Level required) {
+    bool stable = (provided & required) == required;
 
-    if (provided.level != UNDECLARED && !isDeclaredLevel(provided.level)) {
-        ALOGE("Unknown stability when checking interface stability %d.",
-              provided.level);
+    if (provided != UNDECLARED && !isDeclaredLevel(provided)) {
+        ALOGE("Unknown stability when checking interface stability %d.", provided);
 
         stable = false;
     }
@@ -210,11 +170,11 @@ bool Stability::check(Category provided, Level required) {
     return stable;
 }
 
-bool Stability::isDeclaredLevel(Level stability) {
+bool Stability::isDeclaredLevel(int32_t stability) {
     return stability == VENDOR || stability == SYSTEM || stability == VINTF;
 }
 
-std::string Stability::levelString(Level level) {
+std::string Stability::levelString(int32_t level) {
     switch (level) {
         case Level::UNDECLARED: return "undeclared stability";
         case Level::VENDOR: return "vendor stability";
diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h
index 629b565985..ce4362f763 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -44,10 +44,9 @@ namespace internal {
 //   to old servers, and new servers know how to interpret the 8-byte result,
 //   they can still communicate.
 //
-// Every binder object has a stability level associated with it, and when
-// communicating with a binder, we make sure that the command we sent is one
-// that it knows how to process. The summary of stability of a binder is
-// represented by a Stability::Category object.
+// This class is specifically about (1). (2) is not currently tracked by
+// libbinder for regular binder calls, and everything on the system uses the
+// same copy of libbinder.
 
 class Stability final {
 public:
@@ -128,7 +127,10 @@ private:
 
     static void tryMarkCompilationUnit(IBinder* binder);
 
-    enum Level : uint8_t {
+    // Currently, we use int16_t for Level so that it can fit in BBinder.
+    // However, on the wire, we have 4 bytes reserved for stability, so whenever
+    // we ingest a Level, we always accept an int32_t.
+    enum Level : int16_t {
         UNDECLARED = 0,
 
         VENDOR = 0b000011,
@@ -136,37 +138,6 @@ private:
         VINTF = 0b111111,
     };
 
-    // 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(int16_t representation) {
-            return *reinterpret_cast<Category*>(&representation);
-        }
-        int16_t repr() const { return *reinterpret_cast<const int16_t*>(this); }
-        static inline Category currentFromLevel(Level level);
-
-        bool operator== (const Category& o) const {
-            return repr() == o.repr();
-        }
-        bool operator!= (const Category& o) const {
-            return !(*this == o);
-        }
-
-        std::string debugString();
-
-        // This is the version of the wire protocol associated with the host
-        // process of a particular binder. As the wire protocol changes, if
-        // sending a transaction to a binder with an old version, the Parcel
-        // class must write parcels according to the version documented here.
-        uint8_t version;
-
-        // bitmask of Stability::Level
-        Level level;
-    };
-    static_assert(sizeof(Category) == sizeof(int16_t));
-
     // returns the stability according to how this was built
     static Level getLocalLevel();
 
@@ -179,18 +150,18 @@ private:
       REPR_ALLOW_DOWNGRADE = 2,
     };
     // applies stability to binder if stability level is known
-    __attribute__((warn_unused_result))
-    static status_t setRepr(IBinder* binder, int32_t representation, uint32_t flags);
+    __attribute__((warn_unused_result)) static status_t setRepr(IBinder* binder, int32_t setting,
+                                                                uint32_t flags);
 
     // get stability information as encoded on the wire
-    static Category getCategory(IBinder* binder);
+    static int16_t getRepr(IBinder* binder);
 
     // whether a transaction on binder is allowed, if the transaction
     // is done from a context with a specific stability level
-    static bool check(Category provided, Level required);
+    static bool check(int16_t provided, Level required);
 
-    static bool isDeclaredLevel(Level level);
-    static std::string levelString(Level level);
+    static bool isDeclaredLevel(int32_t level);
+    static std::string levelString(int32_t level);
 
     Stability();
 };