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

Commit 98030530 authored by Android Build Coastguard Worker's avatar Android Build Coastguard Worker
Browse files

Merge cherrypicks of ['googleplex-android-review.googlesource.com/29999961',...

Merge cherrypicks of ['googleplex-android-review.googlesource.com/29999961', 'googleplex-android-review.googlesource.com/29955588', 'googleplex-android-review.googlesource.com/30025713'] into security-aosp-tm-release.

Change-Id: I2b1e4c46fa3c2116a31c277c9938bd3558a9bd3b
parents b56a11e0 0b4de68b
Loading
Loading
Loading
Loading
+1 −2
Original line number Diff line number Diff line
@@ -1443,7 +1443,7 @@ status_t IPCThreadState::freeze(pid_t pid, bool enable, uint32_t timeout_ms) {
    return ret;
}

void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data,
void IPCThreadState::freeBuffer(Parcel* /*parcel*/, const uint8_t* data,
                                size_t /*dataSize*/,
                                const binder_size_t* /*objects*/,
                                size_t /*objectsSize*/)
@@ -1453,7 +1453,6 @@ void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data,
        alog << "Writing BC_FREE_BUFFER for " << data << endl;
    }
    ALOG_ASSERT(data != NULL, "Called with NULL data");
    if (parcel != nullptr) parcel->closeFileDescriptors();
    IPCThreadState* state = self();
    state->mOut.writeInt32(BC_FREE_BUFFER);
    state->mOut.writePointer((uintptr_t)data);
+38 −2
Original line number Diff line number Diff line
@@ -888,6 +888,10 @@ restart_write:
        //printf("Writing %ld bytes, padded to %ld\n", len, padded);
        uint8_t* const data = mData+mDataPos;

        if (status_t status = validateReadData(mDataPos + padded); status != OK) {
            return nullptr; // drops status
        }

        // Need to pad at end?
        if (padded != len) {
#if BYTE_ORDER == BIG_ENDIAN
@@ -1405,6 +1409,10 @@ status_t Parcel::writeObject(const flat_binder_object& val, bool nullMetaData)
    const bool enoughObjects = mObjectsSize < mObjectsCapacity;
    if (enoughData && enoughObjects) {
restart_write:
        if (status_t status = validateReadData(mDataPos + sizeof(val)); status != OK) {
            return status;
        }

        *reinterpret_cast<flat_binder_object*>(mData+mDataPos) = val;

        // remember if it's a file descriptor
@@ -1621,6 +1629,10 @@ status_t Parcel::writeAligned(T val) {

    if ((mDataPos+sizeof(val)) <= mDataCapacity) {
restart_write:
        if (status_t status = validateReadData(mDataPos + sizeof(val)); status != OK) {
            return status;
        }

        memcpy(mData + mDataPos, &val, sizeof(val));
        return finishWrite(sizeof(val));
    }
@@ -2193,11 +2205,15 @@ const flat_binder_object* Parcel::readObject(bool nullMetaData) const

void Parcel::closeFileDescriptors()
{
    truncateFileDescriptors(0);
}

void Parcel::truncateFileDescriptors(size_t newObjectsSize) {
    size_t i = mObjectsSize;
    if (i > 0) {
        //ALOGI("Closing file descriptors for %zu objects...", i);
    }
    while (i > 0) {
    while (i > newObjectsSize) {
        i--;
        const flat_binder_object* flat
            = reinterpret_cast<flat_binder_object*>(mData+mObjects[i]);
@@ -2345,6 +2361,7 @@ void Parcel::freeDataNoInit()
    if (mOwner) {
        LOG_ALLOC("Parcel %p: freeing other owner data", this);
        //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
        closeFileDescriptors();
        mOwner(this, mData, mDataSize, mObjects, mObjectsSize);
    } else {
        LOG_ALLOC("Parcel %p: freeing allocated data", this);
@@ -2370,6 +2387,14 @@ status_t Parcel::growData(size_t len)
        return BAD_VALUE;
    }

    if (mDataPos > mDataSize) {
        // b/370831157 - this case used to abort. We also don't expect mDataPos < mDataSize, but
        // this would only waste a bit of memory, so it's okay.
        ALOGE("growData only expected at the end of a Parcel. pos: %zu, size: %zu, capacity: %zu",
              mDataPos, len, mDataCapacity);
        return BAD_VALUE;
    }

    if (len > SIZE_MAX - mDataSize) return NO_MEMORY; // overflow
    if (mDataSize + len > SIZE_MAX / 3) return NO_MEMORY; // overflow
    size_t newSize = ((mDataSize+len)*3)/2;
@@ -2460,8 +2485,9 @@ status_t Parcel::continueWrite(size_t desired)
        if (desired == 0) {
            objectsSize = 0;
        } else {
            validateReadData(mDataSize); // hack to sort the objects
            while (objectsSize > 0) {
                if (mObjects[objectsSize-1] < desired)
                if (mObjects[objectsSize-1] + sizeof(flat_binder_object) <= desired)
                    break;
                objectsSize--;
            }
@@ -2506,8 +2532,18 @@ status_t Parcel::continueWrite(size_t desired)
        }
        if (objects && mObjects) {
            memcpy(objects, mObjects, objectsSize*sizeof(binder_size_t));
            // All FDs are owned when `mOwner`, even when `cookie == 0`. When
            // we switch to `!mOwner`, we need to explicitly mark the FDs as
            // owned.
            for (size_t i = 0; i < objectsSize; i++) {
                flat_binder_object* flat = reinterpret_cast<flat_binder_object*>(data + objects[i]);
                if (flat->hdr.type == BINDER_TYPE_FD) {
                    flat->cookie = 1;
                }
            }
        }
        //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());
        truncateFileDescriptors(objectsSize);
        mOwner(this, mData, mDataSize, mObjects, mObjectsSize);
        mOwner = nullptr;

+3 −0
Original line number Diff line number Diff line
@@ -592,6 +592,9 @@ public:
    void                print(TextOutput& to, uint32_t flags = 0) const;

private:
    // Close all file descriptors in the parcel at object positions >= newObjectsSize.
    __attribute__((__visibility__("hidden"))) void truncateFileDescriptors(size_t newObjectsSize);

    typedef void        (*release_func)(Parcel* parcel,
                                        const uint8_t* data, size_t dataSize,
                                        const binder_size_t* objects, size_t objectsSize);
+162 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include <android-base/logging.h>
#include <android-base/properties.h>
#include <android-base/result-gmock.h>
#include <android-base/result.h>
@@ -42,6 +43,7 @@

#include <linux/sched.h>
#include <sys/epoll.h>
#include <sys/mman.h>
#include <sys/prctl.h>
#include <sys/socket.h>
#include <sys/un.h>
@@ -56,6 +58,7 @@ using namespace std::string_literals;
using namespace std::chrono_literals;
using android::base::testing::HasValue;
using android::base::testing::Ok;
using android::base::unique_fd;
using testing::ExplainMatchResult;
using testing::Matcher;
using testing::Not;
@@ -104,6 +107,8 @@ enum BinderLibTestTranscationCode {
    BINDER_LIB_TEST_LINK_DEATH_TRANSACTION,
    BINDER_LIB_TEST_WRITE_FILE_TRANSACTION,
    BINDER_LIB_TEST_WRITE_PARCEL_FILE_DESCRIPTOR_TRANSACTION,
    BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION,
    BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION,
    BINDER_LIB_TEST_EXIT_TRANSACTION,
    BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION,
    BINDER_LIB_TEST_GET_PTR_SIZE_TRANSACTION,
@@ -437,6 +442,35 @@ class TestDeathRecipient : public IBinder::DeathRecipient, public BinderLibTestE
        };
};

ssize_t countFds() {
    DIR* dir = opendir("/proc/self/fd/");
    if (dir == nullptr) return -1;
    ssize_t ret = 0;
    dirent* ent;
    while ((ent = readdir(dir)) != nullptr) ret++;
    closedir(dir);
    return ret;
}

struct FdLeakDetector {
    int startCount;

    FdLeakDetector() {
        // This log statement is load bearing. We have to log something before
        // counting FDs to make sure the logging system is initialized, otherwise
        // the sockets it opens will look like a leak.
        ALOGW("FdLeakDetector counting FDs.");
        startCount = countFds();
    }
    ~FdLeakDetector() {
        int endCount = countFds();
        if (startCount != endCount) {
            ADD_FAILURE() << "fd count changed (" << startCount << " -> " << endCount
                          << ") fd leak?";
        }
    }
};

TEST_F(BinderLibTest, CannotUseBinderAfterFork) {
    // EXPECT_DEATH works by forking the process
    EXPECT_DEATH({ ProcessState::self(); }, "libbinder ProcessState can not be used after fork");
@@ -852,6 +886,100 @@ TEST_F(BinderLibTest, PassParcelFileDescriptor) {
    EXPECT_EQ(0, read(read_end.get(), readbuf.data(), datasize));
}

TEST_F(BinderLibTest, RecvOwnedFileDescriptors) {
    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data,
                                 &reply));
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b));
}

// Used to trigger fdsan error (b/239222407).
TEST_F(BinderLibTest, RecvOwnedFileDescriptorsAndWriteInt) {
    GTEST_SKIP() << "triggers fdsan false positive: b/370824489";

    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data,
                                 &reply));
    reply.setDataPosition(reply.dataSize());
    reply.writeInt32(0);
    reply.setDataPosition(0);
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b));
}

// Used to trigger fdsan error (b/239222407).
TEST_F(BinderLibTest, RecvOwnedFileDescriptorsAndTruncate) {
    GTEST_SKIP() << "triggers fdsan false positive: b/370824489";

    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION, data,
                                 &reply));
    reply.setDataSize(reply.dataSize() - sizeof(flat_binder_object));
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(BAD_TYPE, reply.readUniqueFileDescriptor(&b));
}

TEST_F(BinderLibTest, RecvUnownedFileDescriptors) {
    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data,
                                 &reply));
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b));
}

// Used to trigger fdsan error (b/239222407).
TEST_F(BinderLibTest, RecvUnownedFileDescriptorsAndWriteInt) {
    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data,
                                 &reply));
    reply.setDataPosition(reply.dataSize());
    reply.writeInt32(0);
    reply.setDataPosition(0);
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&b));
}

// Used to trigger fdsan error (b/239222407).
TEST_F(BinderLibTest, RecvUnownedFileDescriptorsAndTruncate) {
    FdLeakDetector fd_leak_detector;

    Parcel data;
    Parcel reply;
    EXPECT_EQ(NO_ERROR,
              m_server->transact(BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION, data,
                                 &reply));
    reply.setDataSize(reply.dataSize() - sizeof(flat_binder_object));
    unique_fd a, b;
    EXPECT_EQ(OK, reply.readUniqueFileDescriptor(&a));
    EXPECT_EQ(BAD_TYPE, reply.readUniqueFileDescriptor(&b));
}

TEST_F(BinderLibTest, PromoteLocal) {
    sp<IBinder> strong = new BBinder();
    wp<IBinder> weak = strong;
@@ -1600,6 +1728,40 @@ public:
                if (ret != size) return UNKNOWN_ERROR;
                return NO_ERROR;
            }
            case BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_OWNED_TRANSACTION: {
                unique_fd fd1(memfd_create("memfd1", MFD_CLOEXEC));
                if (!fd1.ok()) {
                    PLOG(ERROR) << "memfd_create failed";
                    return UNKNOWN_ERROR;
                }
                unique_fd fd2(memfd_create("memfd2", MFD_CLOEXEC));
                if (!fd2.ok()) {
                    PLOG(ERROR) << "memfd_create failed";
                    return UNKNOWN_ERROR;
                }
                status_t ret;
                ret = reply->writeFileDescriptor(fd1.release(), true);
                if (ret != NO_ERROR) {
                    return ret;
                }
                ret = reply->writeFileDescriptor(fd2.release(), true);
                if (ret != NO_ERROR) {
                    return ret;
                }
                return NO_ERROR;
            }
            case BINDER_LIB_TEST_GET_FILE_DESCRIPTORS_UNOWNED_TRANSACTION: {
                status_t ret;
                ret = reply->writeFileDescriptor(STDOUT_FILENO, false);
                if (ret != NO_ERROR) {
                    return ret;
                }
                ret = reply->writeFileDescriptor(STDERR_FILENO, false);
                if (ret != NO_ERROR) {
                    return ret;
                }
                return NO_ERROR;
            }
            case BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION:
                alarm(10);
                return NO_ERROR;