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

Commit 1519b984 authored by Andrei Homescu's avatar Andrei Homescu
Browse files

libbinder: remove gMaxFds from Parcel 

libbinder uses getrlimit to get the maximum number of file
descriptors and uses that limit to validate Flattenables.
This syscall is specific to Linux and Mac OS, so this patch
removes it and the gMaxFds variable. Parcel operations for
Flattenables with too many fds should fail anyway when
duplicating the descriptors using fcntl. This adds a test
to binderLibTest for that case.

Bug: 224644083
Test: atest binderLibTest
Change-Id: I7e969180c165c7ebed74c8a97098eb2265e0305c
parent 34f8fc85

libs/binder/Parcel.cpp

+4 −15
Original line number Original line Diff line number Diff line
@@ -87,7 +87,8 @@ static_assert(sizeof(Parcel) == 60); 
static std::atomic<size_t> gParcelGlobalAllocCount;

static std::atomic<size_t> gParcelGlobalAllocCount;

static std::atomic<size_t> gParcelGlobalAllocSize;

static std::atomic<size_t> gParcelGlobalAllocSize;





static size_t gMaxFds = 0;

// Maximum number of file descriptors per Parcel.

constexpr size_t kMaxFds = 1024;





// Maximum size of a blob to transfer in-place.

// Maximum size of a blob to transfer in-place.

static const size_t BLOB_INPLACE_LIMIT = 16 * 1024;

static const size_t BLOB_INPLACE_LIMIT = 16 * 1024;
@@ -1416,7 +1417,7 @@ status_t Parcel::write(const FlattenableHelperInterface& val) 
    const size_t len = val.getFlattenedSize();

    const size_t len = val.getFlattenedSize();

    const size_t fd_count = val.getFdCount();

    const size_t fd_count = val.getFdCount();





    if ((len > INT32_MAX) || (fd_count >= gMaxFds)) {

    if ((len > INT32_MAX) || (fd_count > kMaxFds)) {

        // don't accept size_t values which may have come from an

        // don't accept size_t values which may have come from an

        // inadvertent conversion from a negative int.

        // inadvertent conversion from a negative int.

        return BAD_VALUE;

        return BAD_VALUE;
@@ -2158,7 +2159,7 @@ status_t Parcel::read(FlattenableHelperInterface& val) const 
    const size_t len = this->readInt32();

    const size_t len = this->readInt32();

    const size_t fd_count = this->readInt32();

    const size_t fd_count = this->readInt32();





    if ((len > INT32_MAX) || (fd_count >= gMaxFds)) {

    if ((len > INT32_MAX) || (fd_count > kMaxFds)) {

        // don't accept size_t values which may have come from an

        // don't accept size_t values which may have come from an

        // inadvertent conversion from a negative int.

        // inadvertent conversion from a negative int.

        return BAD_VALUE;

        return BAD_VALUE;
@@ -2747,18 +2748,6 @@ void Parcel::initState() 
    mAllowFds = true;

    mAllowFds = true;

    mDeallocZero = false;

    mDeallocZero = false;

    mOwner = nullptr;

    mOwner = nullptr;



    // racing multiple init leads only to multiple identical write

    if (gMaxFds == 0) {

        struct rlimit result;

        if (!getrlimit(RLIMIT_NOFILE, &result)) {

            gMaxFds = (size_t)result.rlim_cur;

            //ALOGI("parcel fd limit set to %zu", gMaxFds);

        } else {

            ALOGW("Unable to getrlimit: %s", strerror(errno));

            gMaxFds = 1024;

        }

    }

}

}





void Parcel::scanForFds() const {

void Parcel::scanForFds() const {

libs/binder/tests/binderLibTest.cpp

+48 −0
Original line number Original line Diff line number Diff line
@@ -30,6 +30,7 @@ 
#include <android-base/properties.h>

#include <android-base/properties.h>

#include <android-base/result-gmock.h>

#include <android-base/result-gmock.h>

#include <android-base/result.h>

#include <android-base/result.h>

#include <android-base/scopeguard.h>

#include <android-base/strings.h>

#include <android-base/strings.h>

#include <android-base/unique_fd.h>

#include <android-base/unique_fd.h>

#include <binder/Binder.h>

#include <binder/Binder.h>
@@ -1232,6 +1233,53 @@ TEST_F(BinderLibTest, GotSid) { 
    EXPECT_THAT(server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr), StatusEq(OK));

    EXPECT_THAT(server->transact(BINDER_LIB_TEST_CAN_GET_SID, data, nullptr), StatusEq(OK));

}

}





struct TooManyFdsFlattenable : Flattenable<TooManyFdsFlattenable> {

    TooManyFdsFlattenable(size_t fdCount) : mFdCount(fdCount) {}



    // Flattenable protocol

    size_t getFlattenedSize() const {

        // Return a valid non-zero size here so we don't get an unintended

        // BAD_VALUE from Parcel::write

        return 16;

    }

    size_t getFdCount() const { return mFdCount; }

    status_t flatten(void *& /*buffer*/, size_t & /*size*/, int *&fds, size_t &count) const {

        for (size_t i = 0; i < count; i++) {

            fds[i] = STDIN_FILENO;

        }

        return NO_ERROR;

    }

    status_t unflatten(void const *& /*buffer*/, size_t & /*size*/, int const *& /*fds*/,

                       size_t & /*count*/) {

        /* This doesn't get called */

        return NO_ERROR;

    }



    size_t mFdCount;

};



TEST_F(BinderLibTest, TooManyFdsFlattenable) {

    rlimit origNofile;

    int ret = getrlimit(RLIMIT_NOFILE, &origNofile);

    ASSERT_EQ(0, ret);



    // Restore the original file limits when the test finishes

    base::ScopeGuard guardUnguard([&]() { setrlimit(RLIMIT_NOFILE, &origNofile); });



    rlimit testNofile = {1024, 1024};

    ret = setrlimit(RLIMIT_NOFILE, &testNofile);

    ASSERT_EQ(0, ret);



    Parcel parcel;

    // Try to write more file descriptors than supported by the OS

    TooManyFdsFlattenable tooManyFds1(1024);

    EXPECT_THAT(parcel.write(tooManyFds1), StatusEq(-EMFILE));



    // Try to write more file descriptors than the internal limit

    TooManyFdsFlattenable tooManyFds2(1025);

    EXPECT_THAT(parcel.write(tooManyFds2), StatusEq(BAD_VALUE));

}



TEST(ServiceNotifications, Unregister) {

TEST(ServiceNotifications, Unregister) {

    auto sm = defaultServiceManager();

    auto sm = defaultServiceManager();

    using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback;

    using LocalRegistrationCallback = IServiceManager::LocalRegistrationCallback;