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

Commit 10b19f86 authored by Steven Moreland's avatar Steven Moreland Committed by android-build-team Robot
Browse files

libbinder_ndk: fix failure when dump/shell are unset 

People directly using libbinder_ndk functions who didn't create a debug
dump function function would fail to initialize that pointer, and
potentially crash. Those who didn't create a shell function were
guaranteed to crash. This wasn't noticed because the C++ wrappers which
are the recommended way to use libbinder_ndk always set these functions.

Bug: 161812320
Test: unit tests

Merged-In: I1f6909531bc640097f3f48c4a558fd03f2fa62cb
Change-Id: I1f6909531bc640097f3f48c4a558fd03f2fa62cb
(cherry picked from commit deb53467)
parent d04d08d1

libs/binder/ndk/ibinder.cpp

+1 −1
Original line number Diff line number Diff line
@@ -161,7 +161,7 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce 


        binder_status_t status = getClass()->onTransact(this, code, &in, &out);

        return PruneStatusT(status);

    } else if (code == SHELL_COMMAND_TRANSACTION) {

    } else if (code == SHELL_COMMAND_TRANSACTION && getClass()->handleShellCommand != nullptr) {

        int in = data.readFileDescriptor();

        int out = data.readFileDescriptor();

        int err = data.readFileDescriptor();

libs/binder/ndk/ibinder_internal.h

+5 −5
Original line number Diff line number Diff line
@@ -110,13 +110,13 @@ struct AIBinder_Class { 
    const ::android::String16& getInterfaceDescriptor() const { return mInterfaceDescriptor; }



    // required to be non-null, implemented for every class

    const AIBinder_Class_onCreate onCreate;

    const AIBinder_Class_onDestroy onDestroy;

    const AIBinder_Class_onTransact onTransact;

    const AIBinder_Class_onCreate onCreate = nullptr;

    const AIBinder_Class_onDestroy onDestroy = nullptr;

    const AIBinder_Class_onTransact onTransact = nullptr;



    // optional methods for a class

    AIBinder_onDump onDump;

    AIBinder_handleShellCommand handleShellCommand;

    AIBinder_onDump onDump = nullptr;

    AIBinder_handleShellCommand handleShellCommand = nullptr;



   private:

    // This must be a String16 since BBinder virtual getInterfaceDescriptor returns a reference to

libs/binder/ndk/test/iface.cpp

+11 −1
Original line number Diff line number Diff line
@@ -118,7 +118,7 @@ IFoo::~IFoo() { 
    AIBinder_Weak_delete(mWeakBinder);

}



binder_status_t IFoo::addService(const char* instance) {

AIBinder* IFoo::getBinder() {

    AIBinder* binder = nullptr;



    if (mWeakBinder != nullptr) {
@@ -132,8 +132,18 @@ binder_status_t IFoo::addService(const char* instance) { 
            AIBinder_Weak_delete(mWeakBinder);

        }

        mWeakBinder = AIBinder_Weak_new(binder);



        // WARNING: it is important that this class does not implement debug or

        // shell functions because it does not use special C++ wrapper

        // functions, and so this is how we test those functions.

    }



    return binder;

}



binder_status_t IFoo::addService(const char* instance) {

    AIBinder* binder = getBinder();



    binder_status_t status = AServiceManager_addService(binder, instance);

    // Strong references we care about kept by remote process

    AIBinder_decStrong(binder);

libs/binder/ndk/test/include/iface/iface.h

+3 −0
Original line number Diff line number Diff line
@@ -30,6 +30,9 @@ class IFoo : public virtual ::android::RefBase { 


    static AIBinder_Class* kClass;



    // binder representing this interface with one reference count

    AIBinder* getBinder();



    // Takes ownership of IFoo

    binder_status_t addService(const char* instance);

    static ::android::sp<IFoo> getService(const char* instance, AIBinder** outBinder = nullptr);

libs/binder/ndk/test/libbinder_ndk_unit_test.cpp

+20 −0
Original line number Diff line number Diff line
@@ -126,6 +126,26 @@ TEST(NdkBinder, CheckServiceThatDoesExist) { 
    AIBinder_decStrong(binder);

}



TEST(NdkBinder, UnimplementedDump) {

    sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);

    ASSERT_NE(foo, nullptr);

    AIBinder* binder = foo->getBinder();

    EXPECT_EQ(OK, AIBinder_dump(binder, STDOUT_FILENO, nullptr, 0));

    AIBinder_decStrong(binder);

}



TEST(NdkBinder, UnimplementedShell) {

    // libbinder_ndk doesn't support calling shell, so we are calling from the

    // libbinder across processes to the NDK service which doesn't implement

    // shell

    static const sp<android::IServiceManager> sm(android::defaultServiceManager());

    sp<IBinder> testService = sm->getService(String16(IFoo::kSomeInstanceName));



    Vector<String16> argsVec;

    EXPECT_EQ(OK, IBinder::shellCommand(testService, STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO,

                                        argsVec, nullptr, nullptr));

}



TEST(NdkBinder, DoubleNumber) {

    sp<IFoo> foo = IFoo::getService(IFoo::kSomeInstanceName);

    ASSERT_NE(foo, nullptr);