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

Commit 7227c8a7 authored by Steven Moreland's avatar Steven Moreland
Browse files

libbinder: onBinder* respect termination 

Previously, these would leak binder objects if transactions occurred on
already terminated RpcState objects.

Fixes: 189345133
Test: binder_parcel_fuzzer (w/ leak repro), binderRpcTest
Change-Id: I68f86bf0656dd316691634d4fc411e6cac361449
parent e81fe829

libs/binder/Parcel.cpp

+4 −3
Original line number Diff line number Diff line
@@ -283,9 +283,10 @@ status_t Parcel::unflattenBinder(sp<IBinder>* out) const 


        if (isNull & 1) {

            auto addr = RpcAddress::zero();

            status_t status = addr.readFromParcel(*this);

            if (status != OK) return status;

            binder = mSession->state()->onBinderEntering(mSession, addr);

            if (status_t status = addr.readFromParcel(*this); status != OK) return status;

            if (status_t status = mSession->state()->onBinderEntering(mSession, addr, &binder);

                status != OK)

                return status;

        }



        return finishUnflattenBinder(binder, out);

libs/binder/RpcState.cpp

+12 −8
Original line number Diff line number Diff line
@@ -61,6 +61,7 @@ status_t RpcState::onBinderLeaving(const sp<RpcSession>& session, const sp<IBind 
    }



    std::lock_guard<std::mutex> _l(mNodeMutex);

    if (mTerminated) return DEAD_OBJECT;



    // TODO(b/182939933): maybe move address out of BpBinder, and keep binder->address map

    // in RpcState
@@ -95,11 +96,13 @@ status_t RpcState::onBinderLeaving(const sp<RpcSession>& session, const sp<IBind 
    return OK;

}



sp<IBinder> RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddress& address) {

status_t RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcAddress& address,

                                    sp<IBinder>* out) {

    std::unique_lock<std::mutex> _l(mNodeMutex);

    if (mTerminated) return DEAD_OBJECT;



    if (auto it = mNodeForAddress.find(address); it != mNodeForAddress.end()) {

        sp<IBinder> binder = it->second.binder.promote();

        *out = it->second.binder.promote();



        // implicitly have strong RPC refcount, since we received this binder

        it->second.timesRecd++;
@@ -111,7 +114,7 @@ sp<IBinder> RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcA 
        // immediately, we wait to send the last one in BpBinder::onLastDecStrong.

        (void)session->sendDecStrong(address);



        return binder;

        return OK;

    }



    auto&& [it, inserted] = mNodeForAddress.insert({address, BinderNode{}});
@@ -119,10 +122,9 @@ sp<IBinder> RpcState::onBinderEntering(const sp<RpcSession>& session, const RpcA 


    // Currently, all binders are assumed to be part of the same session (no

    // device global binders in the RPC world).

    sp<IBinder> binder = BpBinder::create(session, it->first);

    it->second.binder = binder;

    it->second.binder = *out = BpBinder::create(session, it->first);

    it->second.timesRecd = 1;

    return binder;

    return OK;

}



size_t RpcState::countBinders() {
@@ -556,12 +558,14 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R 
    sp<IBinder> target;

    if (!addr.isZero()) {

        if (!targetRef) {

            target = onBinderEntering(session, addr);

            replyStatus = onBinderEntering(session, addr, &target);

        } else {

            target = targetRef;

        }



        if (target == nullptr) {

        if (replyStatus != OK) {

            // do nothing

        } else if (target == nullptr) {

            // This can happen if the binder is remote in this process, and

            // another thread has called the last decStrong on this binder.

            // However, for local binders, it indicates a misbehaving client

libs/binder/RpcState.h

+2 −1
Original line number Diff line number Diff line
@@ -81,7 +81,8 @@ public: 
     * to the process, if this process already has one, or it takes ownership of

     * that refcount

     */

    sp<IBinder> onBinderEntering(const sp<RpcSession>& session, const RpcAddress& address);

    [[nodiscard]] status_t onBinderEntering(const sp<RpcSession>& session,

                                            const RpcAddress& address, sp<IBinder>* out);



    size_t countBinders();

    void dump();