Set stability for unset interfaces.
This specifically has handwritten interfaces in mind. This CL ensures
that if there is a handwritten interface, it'll be marked with 'local'
stability before it is sent out. This means that a handwritten interface
won't accidentally be interpretted as a stable interface.
Bug: 136027762
Test: boot
Merged-In: I97e3e7c72f99e12dbf00996ff0c68fb683f3c494
Change-Id: I97e3e7c72f99e12dbf00996ff0c68fb683f3c494
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp
index 51ca7b4..a2333ae 100644
--- a/libs/binder/Parcel.cpp
+++ b/libs/binder/Parcel.cpp
@@ -167,6 +167,8 @@
status_t Parcel::finishFlattenBinder(
const sp<IBinder>& /*binder*/, const flat_binder_object& flat)
{
+ // internal::Stability::tryMarkCompilationUnit(binder.get());
+
status_t status = writeObject(flat, false);
if (status != OK) return status;
@@ -183,11 +185,11 @@
// status_t status = readInt32(&stability);
// if (status != OK) return status;
- // if (!internal::Stability::check(stability, mRequiredStability)) {
+ // if (binder != nullptr && !internal::Stability::check(stability, mRequiredStability)) {
// return BAD_TYPE;
// }
- // status = internal::Stability::set(binder.get(), stability);
+ // status = internal::Stability::set(binder.get(), stability, true /*log*/);
// if (status != OK) return status;
*out = binder;
diff --git a/libs/binder/Stability.cpp b/libs/binder/Stability.cpp
index d6d312a..f18bdca 100644
--- a/libs/binder/Stability.cpp
+++ b/libs/binder/Stability.cpp
@@ -13,29 +13,26 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-
#include <binder/Stability.h>
namespace android {
namespace internal {
void Stability::markCompilationUnit(IBinder* binder) {
-#ifdef __ANDROID_VNDK__
-constexpr Stability::Level kLocalStability = Stability::Level::VENDOR;
-#else
-constexpr Stability::Level kLocalStability = Stability::Level::SYSTEM;
-#endif
-
- status_t result = set(binder, kLocalStability);
+ status_t result = set(binder, kLocalStability, true /*log*/);
LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
}
void Stability::markVintf(IBinder* binder) {
- status_t result = set(binder, Level::VINTF);
+ status_t result = set(binder, Level::VINTF, true /*log*/);
LOG_ALWAYS_FATAL_IF(result != OK, "Should only mark known object.");
}
-status_t Stability::set(IBinder* binder, int32_t stability) {
+void Stability::tryMarkCompilationUnit(IBinder* binder) {
+ (void) set(binder, kLocalStability, false /*log*/);
+}
+
+status_t Stability::set(IBinder* binder, int32_t stability, bool log) {
Level currentStability = get(binder);
// null binder is always written w/ 'UNDECLARED' stability
@@ -43,23 +40,26 @@
if (stability == UNDECLARED) {
return OK;
} else {
- ALOGE("Null binder written with stability %s.", stabilityString(stability).c_str());
+ if (log) {
+ ALOGE("Null binder written with stability %s.",
+ stabilityString(stability).c_str());
+ }
return BAD_TYPE;
}
}
if (!isDeclaredStability(stability)) {
- // There are UNDECLARED sets because some binder interfaces don't set their stability, and
- // then UNDECLARED stability is sent on the other side.
- if (stability != UNDECLARED) {
+ if (log) {
ALOGE("Can only set known stability, not %d.", stability);
- return BAD_TYPE;
}
+ return BAD_TYPE;
}
if (currentStability != Level::UNDECLARED && currentStability != stability) {
- ALOGE("Interface being set with %s but it is already marked as %s.",
- stabilityString(stability).c_str(), stabilityString(stability).c_str());
+ if (log) {
+ ALOGE("Interface being set with %s but it is already marked as %s.",
+ stabilityString(stability).c_str(), stabilityString(stability).c_str());
+ }
return BAD_TYPE;
}
diff --git a/libs/binder/include/binder/Stability.h b/libs/binder/include/binder/Stability.h
index 487f480..148def6 100644
--- a/libs/binder/include/binder/Stability.h
+++ b/libs/binder/include/binder/Stability.h
@@ -45,6 +45,8 @@
// up the efficiency level of a binder object. So, we expose the underlying type.
friend ::android::Parcel;
+ static void tryMarkCompilationUnit(IBinder* binder);
+
enum Level : int16_t {
UNDECLARED = 0,
@@ -53,9 +55,15 @@
VINTF = 0b111111,
};
+#ifdef __ANDROID_VNDK__
+ static constexpr Level kLocalStability = Level::VENDOR;
+#else
+ static constexpr Level kLocalStability = Level::SYSTEM;
+#endif
+
// applies stability to binder if stability level is known
__attribute__((warn_unused_result))
- static status_t set(IBinder* binder, int32_t stability);
+ static status_t set(IBinder* binder, int32_t stability, bool log);
static Level get(IBinder* binder);
diff --git a/libs/binder/tests/binderStabilityTest.cpp b/libs/binder/tests/binderStabilityTest.cpp
index 38e7f52..d8159e3 100644
--- a/libs/binder/tests/binderStabilityTest.cpp
+++ b/libs/binder/tests/binderStabilityTest.cpp
@@ -87,30 +87,16 @@
}
};
-void checkNoStabilityServer(const sp<IBinderStabilityTest>& unkemptServer) {
- EXPECT_TRUE(unkemptServer->sendBinder(new BBinder()).isOk());
- EXPECT_TRUE(unkemptServer->sendBinder(getCompilationUnitStability()).isOk());
- EXPECT_TRUE(unkemptServer->sendBinder(getVintfStability()).isOk());
-
- sp<IBinder> out;
- EXPECT_TRUE(unkemptServer->returnNoStabilityBinder(&out).isOk());
- EXPECT_NE(nullptr, out.get());
-
- EXPECT_TRUE(unkemptServer->returnLocalStabilityBinder(&out).isOk());
- EXPECT_NE(nullptr, out.get());
-
- EXPECT_TRUE(unkemptServer->returnVintfStabilityBinder(&out).isOk());
- EXPECT_NE(nullptr, out.get());
-}
-
-void checkLowStabilityServer(const sp<IBinderStabilityTest>& complServer) {
- EXPECT_FALSE(complServer->sendBinder(new BBinder()).isOk());
+void checkLocalStabilityBinder(const sp<IBinderStabilityTest>& complServer) {
+ // this binder should automatically be set to local stability
+ EXPECT_TRUE(complServer->sendBinder(new BBinder()).isOk());
EXPECT_TRUE(complServer->sendBinder(getCompilationUnitStability()).isOk());
EXPECT_TRUE(complServer->sendBinder(getVintfStability()).isOk());
sp<IBinder> out;
- EXPECT_FALSE(complServer->returnNoStabilityBinder(&out).isOk());
- EXPECT_EQ(nullptr, out.get());
+ // should automatically be set to local stability
+ EXPECT_TRUE(complServer->returnNoStabilityBinder(&out).isOk());
+ EXPECT_NE(nullptr, out.get());
EXPECT_TRUE(complServer->returnLocalStabilityBinder(&out).isOk());
EXPECT_NE(nullptr, out.get());
@@ -140,13 +126,15 @@
// or was written by hand.
auto server = BadStabilityTester::getNoStabilityServer();
ASSERT_NE(nullptr, IInterface::asBinder(server)->localBinder());
- checkNoStabilityServer(server);
+
+ // it should be considered to have local stability
+ checkLocalStabilityBinder(server);
}
TEST(BinderStability, LocalLowStabilityServer) {
auto server = BadStabilityTester::getCompilationUnitStabilityServer();
ASSERT_NE(nullptr, IInterface::asBinder(server)->localBinder());
- checkLowStabilityServer(server);
+ checkLocalStabilityBinder(server);
}
TEST(BinderStability, LocalHighStabilityServer) {
@@ -162,7 +150,8 @@
ASSERT_NE(nullptr, remoteServer.get());
ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
- checkNoStabilityServer(remoteServer);
+ // it should be considered to have local stability
+ checkLocalStabilityBinder(remoteServer);
}
TEST(BinderStability, RemoteLowStabilityServer) {
@@ -172,7 +161,7 @@
ASSERT_NE(nullptr, remoteServer.get());
ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());
- checkLowStabilityServer(remoteServer);
+ checkLocalStabilityBinder(remoteServer);
}
TEST(BinderStability, RemoteVintfServer) {