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

Commit 2ccbe3a0 authored by Josh Gao's avatar Josh Gao Committed by Siarhei Vishniakou
Browse files

InputTransport: store fd in a unique_fd. 

Use unique_fd to hold the file descriptor, so that it gets protected
from being closed by someone else by fdsan.

Test: atest libinput_tests inputflinger_tests
Change-Id: I08df199294f9ddd7646c7bcd637b9c035e3c1e12
parent 796dabc1

include/input/InputTransport.h

+7 −7
Original line number Diff line number Diff line
@@ -42,6 +42,8 @@ 
#include <utils/Timers.h>

#include <utils/Vector.h>



#include <android-base/unique_fd.h>



namespace android {

class Parcel;
@@ -166,8 +168,7 @@ protected: 
    virtual ~InputChannel();



public:

    InputChannel() = default;

    InputChannel(const std::string& name, int fd);

    static sp<InputChannel> create(const std::string& name, android::base::unique_fd fd);



    /* Creates a pair of input channels.

     *
@@ -177,7 +178,7 @@ public: 
            sp<InputChannel>& outServerChannel, sp<InputChannel>& outClientChannel);



    inline std::string getName() const { return mName; }

    inline int getFd() const { return mFd; }

    inline int getFd() const { return mFd.get(); }



    /* Sends a message to the other endpoint.

     *
@@ -208,16 +209,15 @@ public: 
    sp<InputChannel> dup() const;



    status_t write(Parcel& out) const;

    status_t read(const Parcel& from);

    static sp<InputChannel> read(const Parcel& from);



    sp<IBinder> getToken() const;

    void setToken(const sp<IBinder>& token);



private:

    void setFd(int fd);



    InputChannel(const std::string& name, android::base::unique_fd fd);

    std::string mName;

    int mFd = -1;

    android::base::unique_fd mFd;



    sp<IBinder> mToken = nullptr;

};

libs/input/IInputFlinger.cpp

+2 −4
Original line number Diff line number Diff line
@@ -92,15 +92,13 @@ status_t BnInputFlinger::onTransact( 
    }

    case REGISTER_INPUT_CHANNEL_TRANSACTION: {

        CHECK_INTERFACE(IInputFlinger, data, reply);

        sp<InputChannel> channel = new InputChannel();

        channel->read(data);

        sp<InputChannel> channel = InputChannel::read(data);

        registerInputChannel(channel);

        break;

    }

    case UNREGISTER_INPUT_CHANNEL_TRANSACTION: {

        CHECK_INTERFACE(IInputFlinger, data, reply);

        sp<InputChannel> channel = new InputChannel();

        channel->read(data);

        sp<InputChannel> channel = InputChannel::read(data);

        unregisterInputChannel(channel);

        break;

    }

libs/input/InputTransport.cpp

+43 −45
Original line number Diff line number Diff line
@@ -227,35 +227,28 @@ void InputMessage::getSanitizedCopy(InputMessage* msg) const { 


// --- InputChannel ---



InputChannel::InputChannel(const std::string& name, int fd) :

        mName(name) {

sp<InputChannel> InputChannel::create(const std::string& name, android::base::unique_fd fd) {

    const int result = fcntl(fd, F_SETFL, O_NONBLOCK);

    if (result != 0) {

        LOG_ALWAYS_FATAL("channel '%s' ~ Could not make socket non-blocking: %s", name.c_str(),

                         strerror(errno));

        return nullptr;

    }

    return new InputChannel(name, std::move(fd));

}



InputChannel::InputChannel(const std::string& name, android::base::unique_fd fd)

      : mName(name), mFd(std::move(fd)) {

#if DEBUG_CHANNEL_LIFECYCLE

    ALOGD("Input channel constructed: name='%s', fd=%d",

            mName.c_str(), fd);

#endif



    setFd(fd);

}



InputChannel::~InputChannel() {

#if DEBUG_CHANNEL_LIFECYCLE

    ALOGD("Input channel destroyed: name='%s', fd=%d",

            mName.c_str(), mFd);

    ALOGD("Input channel destroyed: name='%s', fd=%d", mName.c_str(), mFd.get());

#endif



    ::close(mFd);

}



void InputChannel::setFd(int fd) {

    if (mFd > 0) {

        ::close(mFd);

    }

    mFd = fd;

    if (mFd > 0) {

        int result = fcntl(mFd, F_SETFL, O_NONBLOCK);

        LOG_ALWAYS_FATAL_IF(result != 0, "channel '%s' ~ Could not make socket "

            "non-blocking.  errno=%d", mName.c_str(), errno);

    }

}



status_t InputChannel::openInputChannelPair(const std::string& name,
@@ -276,13 +269,13 @@ status_t InputChannel::openInputChannelPair(const std::string& name, 
    setsockopt(sockets[1], SOL_SOCKET, SO_SNDBUF, &bufferSize, sizeof(bufferSize));

    setsockopt(sockets[1], SOL_SOCKET, SO_RCVBUF, &bufferSize, sizeof(bufferSize));



    std::string serverChannelName = name;

    serverChannelName += " (server)";

    outServerChannel = new InputChannel(serverChannelName, sockets[0]);

    std::string serverChannelName = name + " (server)";

    android::base::unique_fd serverFd(sockets[0]);

    outServerChannel = InputChannel::create(serverChannelName, std::move(serverFd));



    std::string clientChannelName = name;

    clientChannelName += " (client)";

    outClientChannel = new InputChannel(clientChannelName, sockets[1]);

    std::string clientChannelName = name + " (client)";

    android::base::unique_fd clientFd(sockets[1]);

    outClientChannel = InputChannel::create(clientChannelName, std::move(clientFd));

    return OK;

}
@@ -292,7 +285,7 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { 
    msg->getSanitizedCopy(&cleanMsg);

    ssize_t nWrite;

    do {

        nWrite = ::send(mFd, &cleanMsg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL);

        nWrite = ::send(mFd.get(), &cleanMsg, msgLength, MSG_DONTWAIT | MSG_NOSIGNAL);

    } while (nWrite == -1 && errno == EINTR);



    if (nWrite < 0) {
@@ -327,7 +320,7 @@ status_t InputChannel::sendMessage(const InputMessage* msg) { 
status_t InputChannel::receiveMessage(InputMessage* msg) {

    ssize_t nRead;

    do {

        nRead = ::recv(mFd, msg, sizeof(InputMessage), MSG_DONTWAIT);

        nRead = ::recv(mFd.get(), msg, sizeof(InputMessage), MSG_DONTWAIT);

    } while (nRead == -1 && errno == EINTR);



    if (nRead < 0) {
@@ -365,39 +358,44 @@ status_t InputChannel::receiveMessage(InputMessage* msg) { 
}



sp<InputChannel> InputChannel::dup() const {

    int fd = ::dup(getFd());

    return fd >= 0 ? new InputChannel(getName(), fd) : nullptr;

    android::base::unique_fd newFd(::dup(getFd()));

    if (!newFd.ok()) {

        ALOGE("Could not duplicate fd %i for channel %s: %s", getFd(), mName.c_str(),

              strerror(errno));

        return nullptr;

    }

    return InputChannel::create(mName, std::move(newFd));

}





status_t InputChannel::write(Parcel& out) const {

    status_t s = out.writeString8(String8(getName().c_str()));



    status_t s = out.writeCString(getName().c_str());

    if (s != OK) {

        return s;

    }



    s = out.writeStrongBinder(mToken);

    if (s != OK) {

        return s;

    }



    s = out.writeDupFileDescriptor(getFd());



    s = out.writeUniqueFileDescriptor(mFd);

    return s;

}



status_t InputChannel::read(const Parcel& from) {

    mName = from.readString8();

    mToken = from.readStrongBinder();



    int rawFd = from.readFileDescriptor();

    setFd(::dup(rawFd));



    if (mFd < 0) {

        return BAD_VALUE;

sp<InputChannel> InputChannel::read(const Parcel& from) {

    std::string name = from.readCString();

    sp<IBinder> token = from.readStrongBinder();

    android::base::unique_fd rawFd;

    status_t fdResult = from.readUniqueFileDescriptor(&rawFd);

    if (fdResult != OK) {

        return nullptr;

    }



    return OK;

    sp<InputChannel> channel = InputChannel::create(name, std::move(rawFd));

    if (channel != nullptr) {

        channel->setToken(token);

    }

    return channel;

}



sp<IBinder> InputChannel::getToken() const {

libs/input/tests/InputChannel_test.cpp

+18 −9
Original line number Diff line number Diff line
@@ -22,11 +22,12 @@ 
#include <time.h>

#include <errno.h>



#include <binder/Binder.h>

#include <gtest/gtest.h>

#include <input/InputTransport.h>

#include <utils/Timers.h>

#include <utils/StopWatch.h>

#include <utils/StrongPointer.h>

#include <utils/Timers.h>



namespace android {
@@ -43,20 +44,28 @@ TEST_F(InputChannelTest, ConstructorAndDestructor_TakesOwnershipOfFileDescriptor 
    // of a pipe and to check for EPIPE on the other end after the channel is destroyed.

    Pipe pipe;



    sp<InputChannel> inputChannel = new InputChannel("channel name", pipe.sendFd);

    android::base::unique_fd sendFd(pipe.sendFd);



    sp<InputChannel> inputChannel = InputChannel::create("channel name", std::move(sendFd));



    EXPECT_NE(inputChannel, nullptr) << "channel should be successfully created";

    EXPECT_STREQ("channel name", inputChannel->getName().c_str())

            << "channel should have provided name";

    EXPECT_EQ(pipe.sendFd, inputChannel->getFd())

            << "channel should have provided fd";

    EXPECT_NE(-1, inputChannel->getFd()) << "channel should have valid fd";



    inputChannel.clear(); // destroys input channel

    // InputChannel should be the owner of the file descriptor now

    ASSERT_FALSE(sendFd.ok());

}



    EXPECT_EQ(-EPIPE, pipe.readSignal())

            << "channel should have closed fd when destroyed";

TEST_F(InputChannelTest, SetAndGetToken) {

    Pipe pipe;

    sp<InputChannel> channel =

            InputChannel::create("test channel", android::base::unique_fd(pipe.sendFd));

    EXPECT_EQ(channel->getToken(), nullptr);



    // clean up fds of Pipe endpoints that were closed so we don't try to close them again

    pipe.sendFd = -1;

    sp<IBinder> token = new BBinder();

    channel->setToken(token);

    EXPECT_EQ(token, channel->getToken());

}



TEST_F(InputChannelTest, OpenInputChannelPair_ReturnsAPairOfConnectedChannels) {