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

Commit c59b49c3 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge changes I5160e72b,I54ff0c8e 

* changes:
  libbinder: Validate the RPC object positions immediately
  libbinder: Remove Parcel argument from Parcel::release_func
parents ec602d42 694e9732

libs/binder/IPCThreadState.cpp

+9 −16
Original line number Diff line number Diff line
@@ -972,16 +972,13 @@ status_t IPCThreadState::waitForResponse(Parcel *reply, status_t *acquireResult) 
                            freeBuffer);

                    } else {

                        err = *reinterpret_cast<const status_t*>(tr.data.ptr.buffer);

                        freeBuffer(nullptr,

                            reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),

                        freeBuffer(reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),

                                   tr.data_size,

                                   reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),

                                   tr.offsets_size / sizeof(binder_size_t));

                    }

                } else {

                    freeBuffer(nullptr,

                        reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer),

                        tr.data_size,

                    freeBuffer(reinterpret_cast<const uint8_t*>(tr.data.ptr.buffer), tr.data_size,

                               reinterpret_cast<const binder_size_t*>(tr.data.ptr.offsets),

                               tr.offsets_size / sizeof(binder_size_t));

                    continue;
@@ -1473,17 +1470,13 @@ void IPCThreadState::logExtendedError() { 
             ee.id, ee.command, ee.param);

}



void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data,

                                size_t /*dataSize*/,

                                const binder_size_t* /*objects*/,

                                size_t /*objectsSize*/)

{

void IPCThreadState::freeBuffer(const uint8_t* data, size_t /*dataSize*/,

                                const binder_size_t* /*objects*/, size_t /*objectsSize*/) {

    //ALOGI("Freeing parcel %p", &parcel);

    IF_LOG_COMMANDS() {

        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);

libs/binder/Parcel.cpp

+26 −9
Original line number Diff line number Diff line
@@ -2590,6 +2590,22 @@ status_t Parcel::rpcSetDataReference( 


    LOG_ALWAYS_FATAL_IF(session == nullptr);



    if (objectTableSize != ancillaryFds.size()) {

        ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());

        relFunc(data, dataSize, nullptr, 0);

        return BAD_VALUE;

    }

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

        uint32_t minObjectEnd;

        if (__builtin_add_overflow(objectTable[i], sizeof(RpcFields::ObjectType), &minObjectEnd) ||

            minObjectEnd >= dataSize) {

            ALOGE("received out of range object position: %" PRIu32 " (parcel size is %zu)",

                  objectTable[i], dataSize);

            relFunc(data, dataSize, nullptr, 0);

            return BAD_VALUE;

        }

    }



    freeData();

    markForRpc(session);
@@ -2600,12 +2616,6 @@ status_t Parcel::rpcSetDataReference( 
    mDataSize = mDataCapacity = dataSize;

    mOwner = relFunc;



    if (objectTableSize != ancillaryFds.size()) {

        ALOGE("objectTableSize=%zu ancillaryFds.size=%zu", objectTableSize, ancillaryFds.size());

        freeData(); // don't leak mData

        return BAD_VALUE;

    }



    rpcFields->mObjectPositions.reserve(objectTableSize);

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

        rpcFields->mObjectPositions.push_back(objectTable[i]);
@@ -2706,7 +2716,9 @@ void Parcel::freeDataNoInit() 
        LOG_ALLOC("Parcel %p: freeing other owner data", this);

        //ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());

        auto* kernelFields = maybeKernelFields();

        mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,

        // Close FDs before freeing, otherwise they will leak for kernel binder.

        closeFileDescriptors();

        mOwner(mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,

               kernelFields ? kernelFields->mObjectsSize : 0);

    } else {

        LOG_ALLOC("Parcel %p: freeing allocated data", this);
@@ -2892,7 +2904,12 @@ status_t Parcel::continueWrite(size_t desired) 
            memcpy(objects, kernelFields->mObjects, objectsSize * sizeof(binder_size_t));

        }

        // ALOGI("Freeing data ref of %p (pid=%d)", this, getpid());

        mOwner(this, mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,

        if (kernelFields) {

            // TODO(b/239222407): This seems wrong. We should only free FDs when

            // they are in a truncated section of the parcel.

            closeFileDescriptors();

        }

        mOwner(mData, mDataSize, kernelFields ? kernelFields->mObjects : nullptr,

               kernelFields ? kernelFields->mObjectsSize : 0);

        mOwner = nullptr;

libs/binder/RpcState.cpp

+4 −6
Original line number Diff line number Diff line
@@ -586,13 +586,12 @@ status_t RpcState::transactAddress(const sp<RpcSession::RpcConnection>& connecti 
    return waitForReply(connection, session, reply);

}



static void cleanup_reply_data(Parcel* p, const uint8_t* data, size_t dataSize,

                               const binder_size_t* objects, size_t objectsCount) {

    (void)p;

static void cleanup_reply_data(const uint8_t* data, size_t dataSize, const binder_size_t* objects,

                               size_t objectsCount) {

    delete[] const_cast<uint8_t*>(data);

    (void)dataSize;

    LOG_ALWAYS_FATAL_IF(objects != nullptr);

    LOG_ALWAYS_FATAL_IF(objectsCount != 0, "%zu objects remaining", objectsCount);

    (void)objectsCount;

}



status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection,
@@ -799,9 +798,8 @@ status_t RpcState::processTransact( 
                                   std::move(ancillaryFds));

}



static void do_nothing_to_transact_data(Parcel* p, const uint8_t* data, size_t dataSize,

static void do_nothing_to_transact_data(const uint8_t* data, size_t dataSize,

                                        const binder_size_t* objects, size_t objectsCount) {

    (void)p;

    (void)data;

    (void)dataSize;

    (void)objects;

libs/binder/include/binder/IPCThreadState.h

+2 −3
Original line number Diff line number Diff line
@@ -217,9 +217,8 @@ private: 
            void                clearCaller();



    static  void                threadDestructor(void *st);

    static  void                freeBuffer(Parcel* parcel,

                                           const uint8_t* data, size_t dataSize,

                                           const binder_size_t* objects, size_t objectsSize);

    static void freeBuffer(const uint8_t* data, size_t dataSize, const binder_size_t* objects,

                           size_t objectsSize);

    static  void                logExtendedError();



    const   sp<ProcessState>    mProcess;

libs/binder/include/binder/Parcel.h

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



private:

    typedef void        (*release_func)(Parcel* parcel,

                                        const uint8_t* data, size_t dataSize,

                                        const binder_size_t* objects, size_t objectsSize);

    // `objects` and `objectsSize` always 0 for RPC Parcels.

    typedef void (*release_func)(const uint8_t* data, size_t dataSize, const binder_size_t* objects,

                                 size_t objectsSize);



    uintptr_t           ipcData() const;

    size_t              ipcDataSize() const;