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

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

libbinder: transaction includes refcount to binder 

Prevents case where one thread is making a transaction and another
thread clears the ref to this transaction (mainly this is a problem
with oneway transactions). This is something which the binder driver
also does implicitly, but it was missing from the RPC binder
implementation.

Bug: 183140903
Test: binderRpcTest
Change-Id: I4f59ad6094f90e5c95af5febea2780bed29d4c88
parent c1500480

libs/binder/BpBinder.cpp

+2 −1
Original line number Diff line number Diff line
@@ -273,7 +273,8 @@ status_t BpBinder::transact( 


        status_t status;

        if (CC_UNLIKELY(isRpcBinder())) {

            status = rpcSession()->transact(rpcAddress(), code, data, reply, flags);

            status = rpcSession()->transact(sp<IBinder>::fromExisting(this), code, data, reply,

                                            flags);

        } else {

            status = IPCThreadState::self()->transact(binderHandle(), code, data, reply, flags);

        }

libs/binder/RpcSession.cpp

+2 −2
Original line number Diff line number Diff line
@@ -100,12 +100,12 @@ status_t RpcSession::getRemoteMaxThreads(size_t* maxThreads) { 
    return state()->getMaxThreads(connection.fd(), sp<RpcSession>::fromExisting(this), maxThreads);

}



status_t RpcSession::transact(const RpcAddress& address, uint32_t code, const Parcel& data,

status_t RpcSession::transact(const sp<IBinder>& binder, uint32_t code, const Parcel& data,

                              Parcel* reply, uint32_t flags) {

    ExclusiveConnection connection(sp<RpcSession>::fromExisting(this),

                                   (flags & IBinder::FLAG_ONEWAY) ? ConnectionUse::CLIENT_ASYNC

                                                                  : ConnectionUse::CLIENT);

    return state()->transact(connection.fd(), address, code, data,

    return state()->transact(connection.fd(), binder, code, data,

                             sp<RpcSession>::fromExisting(this), reply, flags);

}

libs/binder/RpcState.cpp

+78 −58
Original line number Diff line number Diff line
@@ -253,8 +253,8 @@ sp<IBinder> RpcState::getRootObject(const base::unique_fd& fd, const sp<RpcSessi 
    data.markForRpc(session);

    Parcel reply;



    status_t status = transact(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_ROOT, data, session,

                               &reply, 0);

    status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_ROOT, data,

                                      session, &reply, 0);

    if (status != OK) {

        ALOGE("Error getting root object: %s", statusToString(status).c_str());

        return nullptr;
@@ -269,8 +269,8 @@ status_t RpcState::getMaxThreads(const base::unique_fd& fd, const sp<RpcSession> 
    data.markForRpc(session);

    Parcel reply;



    status_t status = transact(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_MAX_THREADS, data,

                               session, &reply, 0);

    status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_MAX_THREADS,

                                      data, session, &reply, 0);

    if (status != OK) {

        ALOGE("Error getting max threads: %s", statusToString(status).c_str());

        return status;
@@ -294,8 +294,8 @@ status_t RpcState::getSessionId(const base::unique_fd& fd, const sp<RpcSession>& 
    data.markForRpc(session);

    Parcel reply;



    status_t status = transact(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_SESSION_ID, data,

                               session, &reply, 0);

    status_t status = transactAddress(fd, RpcAddress::zero(), RPC_SPECIAL_TRANSACT_GET_SESSION_ID,

                                      data, session, &reply, 0);

    if (status != OK) {

        ALOGE("Error getting session ID: %s", statusToString(status).c_str());

        return status;
@@ -309,9 +309,31 @@ status_t RpcState::getSessionId(const base::unique_fd& fd, const sp<RpcSession>& 
    return OK;

}



status_t RpcState::transact(const base::unique_fd& fd, const RpcAddress& address, uint32_t code,

status_t RpcState::transact(const base::unique_fd& fd, const sp<IBinder>& binder, uint32_t code,

                            const Parcel& data, const sp<RpcSession>& session, Parcel* reply,

                            uint32_t flags) {

    if (!data.isForRpc()) {

        ALOGE("Refusing to send RPC with parcel not crafted for RPC");

        return BAD_TYPE;

    }



    if (data.objectsCount() != 0) {

        ALOGE("Parcel at %p has attached objects but is being used in an RPC call", &data);

        return BAD_TYPE;

    }



    RpcAddress address = RpcAddress::zero();

    if (status_t status = onBinderLeaving(session, binder, &address); status != OK) return status;



    return transactAddress(fd, address, code, data, session, reply, flags);

}



status_t RpcState::transactAddress(const base::unique_fd& fd, const RpcAddress& address,

                                   uint32_t code, const Parcel& data, const sp<RpcSession>& session,

                                   Parcel* reply, uint32_t flags) {

    LOG_ALWAYS_FATAL_IF(!data.isForRpc());

    LOG_ALWAYS_FATAL_IF(data.objectsCount() != 0);



    uint64_t asyncNumber = 0;



    if (!address.isZero()) {
@@ -326,16 +348,6 @@ status_t RpcState::transact(const base::unique_fd& fd, const RpcAddress& address 
        }

    }



    if (!data.isForRpc()) {

        ALOGE("Refusing to send RPC with parcel not crafted for RPC");

        return BAD_TYPE;

    }



    if (data.objectsCount() != 0) {

        ALOGE("Parcel at %p has attached objects but is being used in an RPC call", &data);

        return BAD_TYPE;

    }



    RpcWireTransaction transaction{

            .address = address.viewRawEmbedded(),

            .code = code,
@@ -509,7 +521,7 @@ status_t RpcState::processTransact(const base::unique_fd& fd, const sp<RpcSessio 
        return DEAD_OBJECT;

    }



    return processTransactInternal(fd, session, std::move(transactionData));

    return processTransactInternal(fd, session, std::move(transactionData), nullptr /*targetRef*/);

}



static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,
@@ -522,7 +534,7 @@ static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t d 
}



status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<RpcSession>& session,

                                           CommandData transactionData) {

                                           CommandData transactionData, sp<IBinder>&& targetRef) {

    if (transactionData.size() < sizeof(RpcWireTransaction)) {

        ALOGE("Expecting %zu but got %zu bytes for RpcWireTransaction. Terminating!",

              sizeof(RpcWireTransaction), transactionData.size());
@@ -538,14 +550,12 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R 
    status_t replyStatus = OK;

    sp<IBinder> target;

    if (!addr.isZero()) {

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



        auto it = mNodeForAddress.find(addr);

        if (it == mNodeForAddress.end()) {

            ALOGE("Unknown binder address %s.", addr.toString().c_str());

            replyStatus = BAD_VALUE;

        if (!targetRef) {

            target = onBinderEntering(session, addr);

        } else {

            target = it->second.binder.promote();

            target = targetRef;

        }



        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.
@@ -558,18 +568,25 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R 
            terminate();

            replyStatus = BAD_VALUE;

        } else if (target->localBinder() == nullptr) {

                ALOGE("Transactions can only go to local binders, not address %s. Terminating!",

            ALOGE("Unknown binder address or non-local binder, not address %s. Terminating!",

                  addr.toString().c_str());

            terminate();

            replyStatus = BAD_VALUE;

        } else if (transaction->flags & IBinder::FLAG_ONEWAY) {

                if (transaction->asyncNumber != it->second.asyncNumber) {

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

            auto it = mNodeForAddress.find(addr);

            if (it->second.binder.promote() != target) {

                ALOGE("Binder became invalid during transaction. Bad client? %s",

                      addr.toString().c_str());

                replyStatus = BAD_VALUE;

            } else if (transaction->asyncNumber != it->second.asyncNumber) {

                // we need to process some other asynchronous transaction

                // first

                // TODO(b/183140903): limit enqueues/detect overfill for bad client

                // TODO(b/183140903): detect when an object is deleted when it still has

                //        pending async transactions

                it->second.asyncTodo.push(BinderNode::AsyncTodo{

                        .ref = target,

                        .data = std::move(transactionData),

                        .asyncNumber = transaction->asyncNumber,

                });
@@ -579,7 +596,6 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R 
            }

        }

    }

    }



    Parcel reply;

    reply.markForRpc(session);
@@ -670,13 +686,17 @@ status_t RpcState::processTransactInternal(const base::unique_fd& fd, const sp<R 
                               it->second.asyncNumber, addr.toString().c_str());



                // justification for const_cast (consider avoiding priority_queue):

                // - AsyncTodo operator< doesn't depend on 'data' object

                // - AsyncTodo operator< doesn't depend on 'data' or 'ref' objects

                // - gotta go fast

                CommandData data = std::move(

                        const_cast<BinderNode::AsyncTodo&>(it->second.asyncTodo.top()).data);

                auto& todo = const_cast<BinderNode::AsyncTodo&>(it->second.asyncTodo.top());



                CommandData nextData = std::move(todo.data);

                sp<IBinder> nextRef = std::move(todo.ref);



                it->second.asyncTodo.pop();

                _l.unlock();

                return processTransactInternal(fd, session, std::move(data));

                return processTransactInternal(fd, session, std::move(nextData),

                                               std::move(nextRef));

            }

        }

        return OK;

libs/binder/RpcState.h

+8 −2
Original line number Diff line number Diff line
@@ -58,9 +58,13 @@ public: 
    status_t getSessionId(const base::unique_fd& fd, const sp<RpcSession>& session,

                          int32_t* sessionIdOut);



    [[nodiscard]] status_t transact(const base::unique_fd& fd, const RpcAddress& address,

    [[nodiscard]] status_t transact(const base::unique_fd& fd, const sp<IBinder>& address,

                                    uint32_t code, const Parcel& data,

                                    const sp<RpcSession>& session, Parcel* reply, uint32_t flags);

    [[nodiscard]] status_t transactAddress(const base::unique_fd& fd, const RpcAddress& address,

                                           uint32_t code, const Parcel& data,

                                           const sp<RpcSession>& session, Parcel* reply,

                                           uint32_t flags);

    [[nodiscard]] status_t sendDecStrong(const base::unique_fd& fd, const RpcAddress& address);

    [[nodiscard]] status_t getAndExecuteCommand(const base::unique_fd& fd,

                                                const sp<RpcSession>& session);
@@ -129,7 +133,8 @@ private: 
                                           const RpcWireHeader& command);

    [[nodiscard]] status_t processTransactInternal(const base::unique_fd& fd,

                                                   const sp<RpcSession>& session,

                                                   CommandData transactionData);

                                                   CommandData transactionData,

                                                   sp<IBinder>&& targetRef);

    [[nodiscard]] status_t processDecStrong(const base::unique_fd& fd,

                                            const sp<RpcSession>& session,

                                            const RpcWireHeader& command);
@@ -165,6 +170,7 @@ private: 


        // async transaction queue, _only_ for local binder

        struct AsyncTodo {

            sp<IBinder> ref;

            CommandData data;

            uint64_t asyncNumber = 0;

libs/binder/include/binder/RpcSession.h

+1 −1
Original line number Diff line number Diff line
@@ -83,7 +83,7 @@ public: 
     */

    status_t getRemoteMaxThreads(size_t* maxThreads);



    [[nodiscard]] status_t transact(const RpcAddress& address, uint32_t code, const Parcel& data,

    [[nodiscard]] status_t transact(const sp<IBinder>& binder, uint32_t code, const Parcel& data,

                                    Parcel* reply, uint32_t flags);

    [[nodiscard]] status_t sendDecStrong(const RpcAddress& address);