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

Commit b8a40de4 authored by Steven Moreland's avatar Steven Moreland Committed by Gerrit Code Review
Browse files

Merge changes I7648ff04,Ie8012b1c,I3e655277

* changes:
  libbinder: RpcServer privatize acceptOne
  libbinder: FdTrigger encapsulate poll
  libbinder: move FdTrigger to RpcSession
parents de10257e a0854670
Loading
Loading
Loading
Loading
+3 −36
Original line number Diff line number Diff line
@@ -16,14 +16,12 @@

#define LOG_TAG "RpcServer"

#include <poll.h>
#include <sys/socket.h>
#include <sys/un.h>

#include <thread>
#include <vector>

#include <android-base/macros.h>
#include <android-base/scopeguard.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
@@ -130,16 +128,6 @@ sp<IBinder> RpcServer::getRootObject() {
    return ret;
}

std::unique_ptr<RpcServer::FdTrigger> RpcServer::FdTrigger::make() {
    auto ret = std::make_unique<RpcServer::FdTrigger>();
    if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) return nullptr;
    return ret;
}

void RpcServer::FdTrigger::trigger() {
    mWrite.reset();
}

void RpcServer::join() {
    LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");

@@ -148,27 +136,12 @@ void RpcServer::join() {
        LOG_ALWAYS_FATAL_IF(!mServer.ok(), "RpcServer must be setup to join.");
        LOG_ALWAYS_FATAL_IF(mShutdownTrigger != nullptr, "Already joined");
        mJoinThreadRunning = true;
        mShutdownTrigger = FdTrigger::make();
        mShutdownTrigger = RpcSession::FdTrigger::make();
        LOG_ALWAYS_FATAL_IF(mShutdownTrigger == nullptr, "Cannot create join signaler");
    }

    while (true) {
        pollfd pfd[]{{.fd = mServer.get(), .events = POLLIN, .revents = 0},
                     {.fd = mShutdownTrigger->readFd().get(), .events = POLLHUP, .revents = 0}};
        int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
        if (ret < 0) {
            ALOGE("Could not poll socket: %s", strerror(errno));
            continue;
        }
        if (ret == 0) {
            continue;
        }
        if (pfd[1].revents & POLLHUP) {
            LOG_RPC_DETAIL("join() exiting because shutdown requested.");
            break;
        }

        (void)acceptOneNoCheck();
    while (mShutdownTrigger->triggerablePollRead(mServer)) {
        (void)acceptOne();
    }

    {
@@ -179,12 +152,6 @@ void RpcServer::join() {
}

bool RpcServer::acceptOne() {
    LOG_ALWAYS_FATAL_IF(!mAgreedExperimental, "no!");
    LOG_ALWAYS_FATAL_IF(!hasServer(), "RpcServer must be setup to acceptOne.");
    return acceptOneNoCheck();
}

bool RpcServer::acceptOneNoCheck() {
    unique_fd clientFd(
            TEMP_FAILURE_RETRY(accept4(mServer.get(), nullptr, nullptr /*length*/, SOCK_CLOEXEC)));

+31 −0
Original line number Diff line number Diff line
@@ -19,10 +19,12 @@
#include <binder/RpcSession.h>

#include <inttypes.h>
#include <poll.h>
#include <unistd.h>

#include <string_view>

#include <android-base/macros.h>
#include <binder/Parcel.h>
#include <binder/RpcServer.h>
#include <binder/Stability.h>
@@ -113,6 +115,35 @@ status_t RpcSession::sendDecStrong(const RpcAddress& address) {
    return state()->sendDecStrong(connection.fd(), address);
}

std::unique_ptr<RpcSession::FdTrigger> RpcSession::FdTrigger::make() {
    auto ret = std::make_unique<RpcSession::FdTrigger>();
    if (!android::base::Pipe(&ret->mRead, &ret->mWrite)) return nullptr;
    return ret;
}

void RpcSession::FdTrigger::trigger() {
    mWrite.reset();
}

bool RpcSession::FdTrigger::triggerablePollRead(base::borrowed_fd fd) {
    while (true) {
        pollfd pfd[]{{.fd = fd.get(), .events = POLLIN, .revents = 0},
                     {.fd = mRead.get(), .events = POLLHUP, .revents = 0}};
        int ret = TEMP_FAILURE_RETRY(poll(pfd, arraysize(pfd), -1));
        if (ret < 0) {
            ALOGE("Could not poll: %s", strerror(errno));
            continue;
        }
        if (ret == 0) {
            continue;
        }
        if (pfd[1].revents & POLLHUP) {
            return false;
        }
        return true;
    }
}

status_t RpcSession::readId() {
    {
        std::lock_guard<std::mutex> _l(mMutex);
+2 −25
Original line number Diff line number Diff line
@@ -134,12 +134,6 @@ public:
     */
    [[nodiscard]] bool shutdown();

    /**
     * Accept one connection on this server. You must have at least one client
     * session before calling this.
     */
    [[nodiscard]] bool acceptOne();

    /**
     * For debugging!
     */
@@ -153,29 +147,12 @@ public:
    void onSessionTerminating(const sp<RpcSession>& session);

private:
    /** This is not a pipe. */
    struct FdTrigger {
        static std::unique_ptr<FdTrigger> make();
        /**
         * poll() on this fd for POLLHUP to get notification when trigger is called
         */
        base::borrowed_fd readFd() const { return mRead; }
        /**
         * Close the write end of the pipe so that the read end receives POLLHUP.
         */
        void trigger();

    private:
        base::unique_fd mWrite;
        base::unique_fd mRead;
    };

    friend sp<RpcServer>;
    RpcServer();

    void establishConnection(sp<RpcServer>&& session, base::unique_fd clientFd);
    bool setupSocketServer(const RpcSocketAddress& address);
    [[nodiscard]] bool acceptOneNoCheck();
    [[nodiscard]] bool acceptOne();

    bool mAgreedExperimental = false;
    size_t mMaxThreads = 1;
@@ -188,7 +165,7 @@ private:
    std::map<int32_t, sp<RpcSession>> mSessions;
    int32_t mSessionIdCounter = 0;
    bool mJoinThreadRunning = false;
    std::unique_ptr<FdTrigger> mShutdownTrigger;
    std::unique_ptr<RpcSession::FdTrigger> mShutdownTrigger;
    std::condition_variable mShutdownCv;
};

+27 −0
Original line number Diff line number Diff line
@@ -112,6 +112,33 @@ private:
    friend RpcServer;
    RpcSession();

    /** This is not a pipe. */
    struct FdTrigger {
        static std::unique_ptr<FdTrigger> make();
        /**
         * poll() on this fd for POLLHUP to get notification when trigger is called
         */
        base::borrowed_fd readFd() const { return mRead; }

        /**
         * Close the write end of the pipe so that the read end receives POLLHUP.
         */
        void trigger();

        /**
         * Poll for a read event.
         *
         * Return:
         *   true - time to read!
         *   false - trigger happened
         */
        bool triggerablePollRead(base::borrowed_fd fd);

    private:
        base::unique_fd mWrite;
        base::unique_fd mRead;
    };

    status_t readId();

    // transfer ownership of thread
+7 −3
Original line number Diff line number Diff line
@@ -61,7 +61,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
    server->iUnderstandThisCodeIsExperimentalAndIWillNotUseItInProduction();
    CHECK(server->setupUnixDomainServer(kSock.c_str()));

    std::thread serverThread([=] { (void)server->acceptOne(); });
    std::thread serverThread([=] { (void)server->join(); });

    sockaddr_un addr{
            .sun_family = AF_UNIX,
@@ -76,8 +76,6 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
                     connect(clientFd.get(), reinterpret_cast<sockaddr*>(&addr), sizeof(addr))))
            << strerror(errno);

    serverThread.join();

    // TODO(b/182938024): fuzz multiple sessions, instead of just one

#if 0
@@ -90,6 +88,12 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {

    clientFd.reset();

    // TODO(185167543): currently this is okay because we only shutdown the one
    // thread, but once we can shutdown other sessions, we'll need to change
    // this behavior in order to make sure all of the input is actually read.
    while (!server->shutdown()) usleep(100);
    serverThread.join();

    // TODO(b/185167543): better way to force a server to shutdown
    while (!server->listSessions().empty() && server->numUninitializedSessions()) {
        usleep(1);