Merge "libbinder: Fix FD handling for queued oneway RPC transactions" am: 4ba1ef38c5 am: 9380acff07 am: e959092df9 am: 3e8ce5740d am: 9401313bf8

Original change: https://android-review.googlesource.com/c/platform/frameworks/native/+/2240016

Change-Id: Ib6965ce80b7aed4b220c735a2857d899b1435039
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

5 files changed, 99 insertions, 0 deletions

diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index c411f4fed3..7067328358 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -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");
 
diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h
index 7aab5eeae7..ac8658540e 100644
--- a/libs/binder/RpcState.h
+++ b/libs/binder/RpcState.h
@@ -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 {
diff --git a/libs/binder/tests/IBinderRpcTest.aidl b/libs/binder/tests/IBinderRpcTest.aidl
index b15a2251ef..a3ed571462 100644
--- a/libs/binder/tests/IBinderRpcTest.aidl
+++ b/libs/binder/tests/IBinderRpcTest.aidl
@@ -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();
 }
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index fbc83046e0..2b7049202a 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -922,6 +922,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";
diff --git a/libs/binder/tests/binderRpcTestCommon.h b/libs/binder/tests/binderRpcTestCommon.h
index 4513d36a65..fddf5f35a2 100644
--- a/libs/binder/tests/binderRpcTestCommon.h
+++ b/libs/binder/tests/binderRpcTestCommon.h
@@ -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