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

Commit 15fff8c1 authored by Yifan Hong's avatar Yifan Hong
Browse files

binder: libbinder_tls ensure to check trigger at least once. 

If the SSL_write / SSL_read finishes in one round, we might have
never checked the fdTrigger. Ensure to check it at least once
before SSL_write / SSL_read.

Test: binderRpcTest
Bug: 190868302
Change-Id: I4588475354025f8f02d3eb998e9390d5babbc860
parent d17353cb

libs/binder/FdTrigger.cpp

+15 −0
Original line number Diff line number Diff line
@@ -59,4 +59,19 @@ status_t FdTrigger::triggerablePoll(base::borrowed_fd fd, int16_t event) { 
    }

}



android::base::Result<bool> FdTrigger::isTriggeredPolled() {

    pollfd pfd{.fd = mRead.get(), .events = 0, .revents = 0};

    int ret = TEMP_FAILURE_RETRY(poll(&pfd, 1, 0));

    if (ret < 0) {

        return android::base::ErrnoError() << "FdTrigger::isTriggeredPolled: Error in poll()";

    }

    if (ret == 0) {

        return false;

    }

    if (pfd.revents & POLLHUP) {

        return true;

    }

    return android::base::Error() << "FdTrigger::isTriggeredPolled: poll() returns " << pfd.revents;

}



} // namespace android

libs/binder/FdTrigger.h

+12 −1
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


#include <memory>



#include <android-base/result.h>

#include <android-base/unique_fd.h>

#include <utils/Errors.h>
@@ -34,7 +35,7 @@ public: 
    void trigger();



    /**

     * Whether this has been triggered.

     * Check whether this has been triggered by checking the write end.

     */

    bool isTriggered();
@@ -49,6 +50,16 @@ public: 
     */

    status_t triggerablePoll(base::borrowed_fd fd, int16_t event);



    /**

     * Check whether this has been triggered by poll()ing the read end.

     *

     * Return:

     *   true - triggered

     *   false - not triggered

     *   error - error when polling

     */

    android::base::Result<bool> isTriggeredPolled();



private:

    base::unique_fd mWrite;

    base::unique_fd mRead;

libs/binder/RpcTransportTls.cpp

+19 −0
Original line number Diff line number Diff line
@@ -304,6 +304,8 @@ public: 
private:

    android::base::unique_fd mSocket;

    Ssl mSsl;



    static status_t isTriggered(FdTrigger* fdTrigger);

};



// Error code is errno.
@@ -324,6 +326,15 @@ Result<size_t> RpcTransportTls::peek(void* buf, size_t size) { 
    return ret;

}



status_t RpcTransportTls::isTriggered(FdTrigger* fdTrigger) {

    auto ret = fdTrigger->isTriggeredPolled();

    if (!ret.ok()) {

        ALOGE("%s: %s", __PRETTY_FUNCTION__, ret.error().message().c_str());

        return ret.error().code() == 0 ? UNKNOWN_ERROR : -ret.error().code();

    }

    return OK;

}



status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const void* data,

                                                  size_t size) {

    auto buffer = reinterpret_cast<const uint8_t*>(data);
@@ -331,6 +342,10 @@ status_t RpcTransportTls::interruptableWriteFully(FdTrigger* fdTrigger, const vo 


    MAYBE_WAIT_IN_FLAKE_MODE;



    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least

    // once. The trigger is also checked via triggerablePoll() after every SSL_write().

    if (status_t status = isTriggered(fdTrigger); status != OK) return status;



    while (buffer < end) {

        size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());

        auto [writeSize, errorQueue] = mSsl.call(SSL_write, buffer, todo);
@@ -358,6 +373,10 @@ status_t RpcTransportTls::interruptableReadFully(FdTrigger* fdTrigger, void* dat 


    MAYBE_WAIT_IN_FLAKE_MODE;



    // Before doing any I/O, check trigger once. This ensures the trigger is checked at least

    // once. The trigger is also checked via triggerablePoll() after every SSL_write().

    if (status_t status = isTriggered(fdTrigger); status != OK) return status;



    while (buffer < end) {

        size_t todo = std::min<size_t>(end - buffer, std::numeric_limits<int>::max());

        auto [readSize, errorQueue] = mSsl.call(SSL_read, buffer, todo);