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

Commit 91a96b64 authored by Steven Moreland's avatar Steven Moreland
Browse files

libbinder_ndk: fix race related to bpbinder 

In a recent CL, I incorrectly stated in a comment (which is now
corrected here) that ABpBinder::onLastStrongRef can assume the
underlying binder also has no reference count. This is simply wrong (and
such a comment should have came with an assertion).

When this was not true (you receive a binder at the same time that
another thread is dropping the last NDK-layer reference to that binder),
the detachObject call was deleting the ABpBinderTag::Value beneath the
code in lookupOrCreateFromBinder that is looking up this binder.

Bug: 192321823
Test: CtsNdkBinderTestCases (more to come)
Change-Id: I71b1081b6d6ca179d17af6bb1e4a1bb3dd4b109b
parent 07ac6eef

libs/binder/ndk/ibinder.cpp

+10 −5
Original line number Diff line number Diff line
@@ -232,13 +232,15 @@ ABpBinder::~ABpBinder() {} 


void ABpBinder::onLastStrongRef(const void* id) {

    // Since ABpBinder is OBJECT_LIFETIME_WEAK, we must remove this weak reference in order for

    // the ABpBinder to be deleted. Since a strong reference to this ABpBinder object should no

    // longer be able to exist at the time of this method call, there is no longer a need to

    // recover it.

    // the ABpBinder to be deleted. Even though we have no more references on the ABpBinder

    // (BpRefBase), the remote object may still exist (for instance, if we

    // receive it from another process, before the ABpBinder is attached).



    ABpBinderTag::Value* value =

            static_cast<ABpBinderTag::Value*>(remote()->detachObject(ABpBinderTag::kId));

    if (value) ABpBinderTag::clean(ABpBinderTag::kId, value, nullptr /*cookie*/);

            static_cast<ABpBinderTag::Value*>(remote()->findObject(ABpBinderTag::kId));

    CHECK_NE(nullptr, value) << "ABpBinder must always be attached";



    remote()->withLock([&]() { value->binder = nullptr; });



    BpRefBase::onLastStrongRef(id);

}
@@ -251,6 +253,9 @@ sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android:: 
        return static_cast<ABBinder*>(binder.get());

    }



    // The following code ensures that for a given binder object (remote or local), if it is not an

    // ABBinder then at most one ABpBinder object exists in a given process representing it.



    auto* value = static_cast<ABpBinderTag::Value*>(binder->findObject(ABpBinderTag::kId));

    if (value == nullptr) {

        value = new ABpBinderTag::Value;