Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 05929556 authored by Steven Moreland's avatar Steven Moreland
Browse files

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
Change-Id: I97e3e7c72f99e12dbf00996ff0c68fb683f3c494
parent 10e4cc4a

libs/binder/Parcel.cpp

+4 −2
Original line number Diff line number Diff line
@@ -170,6 +170,8 @@ static void release_object(const sp<ProcessState>& proc, 
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 Parcel::finishUnflattenBinder( 
    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;

libs/binder/Stability.cpp

+17 −17
Original line number Diff line number Diff line
@@ -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 @@ status_t Stability::set(IBinder* binder, int32_t stability) { 
        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) {

        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;

    }

libs/binder/include/binder/Stability.h

+9 −1
Original line number Diff line number Diff line
@@ -45,6 +45,8 @@ private: 
    // 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 @@ private: 
        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);

libs/binder/tests/binderStabilityTest.cpp

+13 −24
Original line number Diff line number Diff line
@@ -89,30 +89,16 @@ public: 
    }

};



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());
@@ -142,13 +128,15 @@ TEST(BinderStability, LocalNoStabilityServer) { 
    // 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) {
@@ -164,7 +152,8 @@ TEST(BinderStability, RemoteNoStabilityServer) { 
    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) {
@@ -174,7 +163,7 @@ TEST(BinderStability, RemoteLowStabilityServer) { 
    ASSERT_NE(nullptr, remoteServer.get());

    ASSERT_NE(nullptr, IInterface::asBinder(remoteServer)->remoteBinder());



    checkLowStabilityServer(remoteServer);

    checkLocalStabilityBinder(remoteServer);

}



TEST(BinderStability, RemoteVintfServer) {