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

Commit b0221d1a authored by Frederick Mayle's avatar Frederick Mayle
Browse files

libbinder: Fix FD handling for queued oneway RPC transactions 

If a oneway transaction contained FDs and got queued, we'd drop the FDs
and then the Parcel validation would fail with an error once the
transaction was eventually processed.

Bug: 244484370
Test: m libbinder binderRpcTest && out/host/linux-x86/nativetest64/binderRpcTest/binderRpcTest
Change-Id: I781d851d875d496c8b57e3512f3f89c9911e9f3c
parent 12b528b0

libs/binder/RpcState.cpp

+2 −0
Original line number Diff line number Diff line
@@ -886,6 +886,7 @@ processTransactInternalTailCall: 
                it->second.asyncTodo.push(BinderNode::AsyncTodo{

                        .ref = target,

                        .data = std::move(transactionData),

                        .ancillaryFds = std::move(ancillaryFds),

                        .asyncNumber = transaction->asyncNumber,

                });
@@ -1046,6 +1047,7 @@ processTransactInternalTailCall: 


                // reset up arguments

                transactionData = std::move(todo.data);

                ancillaryFds = std::move(todo.ancillaryFds);

                LOG_ALWAYS_FATAL_IF(target != todo.ref,

                                    "async list should be associated with a binder");

libs/binder/RpcState.h

+1 −0
Original line number Diff line number Diff line
@@ -250,6 +250,7 @@ private: 
        struct AsyncTodo {

            sp<IBinder> ref;

            CommandData data;

            std::vector<std::variant<base::unique_fd, base::borrowed_fd>> ancillaryFds;

            uint64_t asyncNumber = 0;



            bool operator<(const AsyncTodo& o) const {

libs/binder/tests/IBinderRpcTest.aidl

+9 −0
Original line number Diff line number Diff line
@@ -71,4 +71,13 @@ interface IBinderRpcTest { 
    ParcelFileDescriptor echoAsFile(@utf8InCpp String content);



    ParcelFileDescriptor concatFiles(in List<ParcelFileDescriptor> files);



    // FDs sent via `blockingSendFdOneway` can be received via

    // `blockingRecvFd`. The handler for `blockingSendFdOneway` will block

    // until the next `blockingRecvFd` call.

    //

    // This is useful for carefully controlling how/when oneway transactions

    // get queued.

    oneway void blockingSendFdOneway(in ParcelFileDescriptor fd);

    ParcelFileDescriptor blockingRecvFd();

}

libs/binder/tests/binderRpcTest.cpp

+39 −0
Original line number Diff line number Diff line
@@ -911,6 +911,45 @@ TEST_P(BinderRpc, OnewayCallDoesNotWait) { 
    EXPECT_LT(epochMsAfter, epochMsBefore + kReallyLongTimeMs);

}



TEST_P(BinderRpc, OnewayCallQueueingWithFds) {

    if (!supportsFdTransport()) {

        GTEST_SKIP() << "Would fail trivially (which is tested elsewhere)";

    }

    if (clientOrServerSingleThreaded()) {

        GTEST_SKIP() << "This test requires multiple threads";

    }



    // This test forces a oneway transaction to be queued by issuing two

    // `blockingSendFdOneway` calls, then drains the queue by issuing two

    // `blockingRecvFd` calls.

    //

    // For more details about the queuing semantics see

    // https://developer.android.com/reference/android/os/IBinder#FLAG_ONEWAY



    auto proc = createRpcTestSocketServerProcess({

            .numThreads = 3,

            .clientFileDescriptorTransportMode = RpcSession::FileDescriptorTransportMode::UNIX,

            .serverSupportedFileDescriptorTransportModes =

                    {RpcSession::FileDescriptorTransportMode::UNIX},

    });



    EXPECT_OK(proc.rootIface->blockingSendFdOneway(

            android::os::ParcelFileDescriptor(mockFileDescriptor("a"))));

    EXPECT_OK(proc.rootIface->blockingSendFdOneway(

            android::os::ParcelFileDescriptor(mockFileDescriptor("b"))));



    android::os::ParcelFileDescriptor fdA;

    EXPECT_OK(proc.rootIface->blockingRecvFd(&fdA));

    std::string result;

    CHECK(android::base::ReadFdToString(fdA.get(), &result));

    EXPECT_EQ(result, "a");



    android::os::ParcelFileDescriptor fdB;

    EXPECT_OK(proc.rootIface->blockingRecvFd(&fdB));

    CHECK(android::base::ReadFdToString(fdB.get(), &result));

    EXPECT_EQ(result, "b");

}



TEST_P(BinderRpc, OnewayCallQueueing) {

    if (clientOrServerSingleThreaded()) {

        GTEST_SKIP() << "This test requires multiple threads";

libs/binder/tests/binderRpcTestCommon.h

+48 −0
Original line number Diff line number Diff line
@@ -167,6 +167,42 @@ static inline base::unique_fd mockFileDescriptor(std::string contents) { 
    return readFd;

}



// A threadsafe channel where writes block until the value is read.

template <typename T>

class HandoffChannel {

public:

    void write(T v) {

        {

            RpcMutexUniqueLock lock(mMutex);

            // Wait for space to send.

            mCvEmpty.wait(lock, [&]() { return !mValue.has_value(); });

            mValue.emplace(std::move(v));

        }

        mCvFull.notify_all();

        RpcMutexUniqueLock lock(mMutex);

        // Wait for it to be taken.

        mCvEmpty.wait(lock, [&]() { return !mValue.has_value(); });

    }



    T read() {

        RpcMutexUniqueLock lock(mMutex);

        if (!mValue.has_value()) {

            mCvFull.wait(lock, [&]() { return mValue.has_value(); });

        }

        T v = std::move(mValue.value());

        mValue.reset();

        lock.unlock();

        mCvEmpty.notify_all();

        return std::move(v);

    }



private:

    RpcMutex mMutex;

    RpcConditionVariable mCvEmpty;

    RpcConditionVariable mCvFull;

    std::optional<T> mValue;

};



using android::binder::Status;



class MyBinderRpcSession : public BnBinderRpcSession {
@@ -374,6 +410,18 @@ public: 
        out->reset(mockFileDescriptor(acc));

        return Status::ok();

    }



    HandoffChannel<android::base::unique_fd> mFdChannel;



    Status blockingSendFdOneway(const android::os::ParcelFileDescriptor& fd) override {

        mFdChannel.write(android::base::unique_fd(fcntl(fd.get(), F_DUPFD_CLOEXEC, 0)));

        return Status::ok();

    }



    Status blockingRecvFd(android::os::ParcelFileDescriptor* fd) override {

        fd->reset(mFdChannel.read());

        return Status::ok();

    }

};



} // namespace android