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

Commit dc5cb62f authored by Sungtak Lee's avatar Sungtak Lee
Browse files

bufferpool2: fix native handle race condition 

Return cloned handle as output parameter in order to use it after
closing the bufferpool connection.

Bug: 138171841
Test: atest CtsMediaTestCases -- --module-arg CtsMediaTestCases:size:small
Change-Id: I4ac142fcb7e3ecdb3fb02792f516b343d71bfc38
parent 107fd5a1

media/bufferpool/2.0/ClientManager.cpp

+23 −2
Original line number Diff line number Diff line
@@ -351,7 +351,17 @@ ResultStatus ClientManager::Impl::allocate( 
        }

        client = it->second;

    }

    return client->allocate(params, handle, buffer);

    native_handle_t *origHandle;

    ResultStatus res = client->allocate(params, &origHandle, buffer);

    if (res != ResultStatus::OK) {

        return res;

    }

    *handle = native_handle_clone(origHandle);

    if (handle == NULL) {

        buffer->reset();

        return ResultStatus::NO_MEMORY;

    }

    return ResultStatus::OK;

}



ResultStatus ClientManager::Impl::receive(
@@ -367,7 +377,18 @@ ResultStatus ClientManager::Impl::receive( 
        }

        client = it->second;

    }

    return client->receive(transactionId, bufferId, timestampUs, handle, buffer);

    native_handle_t *origHandle;

    ResultStatus res = client->receive(

            transactionId, bufferId, timestampUs, &origHandle, buffer);

    if (res != ResultStatus::OK) {

        return res;

    }

    *handle = native_handle_clone(origHandle);

    if (handle == NULL) {

        buffer->reset();

        return ResultStatus::NO_MEMORY;

    }

    return ResultStatus::OK;

}



ResultStatus ClientManager::Impl::postSend(

media/bufferpool/2.0/include/bufferpool/ClientManager.h

+6 −2
Original line number Diff line number Diff line
@@ -104,7 +104,9 @@ struct ClientManager : public IClientManager { 
    ResultStatus flush(ConnectionId connectionId);



    /**

     * Allocates a buffer from the specified connection.

     * Allocates a buffer from the specified connection. The output parameter

     * handle is cloned from the internal handle. So it is safe to use directly,

     * and it should be deleted and destroyed after use.

     *

     * @param connectionId  The id of the connection.

     * @param params        The allocation parameters.
@@ -123,7 +125,9 @@ struct ClientManager : public IClientManager { 
                          std::shared_ptr<BufferPoolData> *buffer);



    /**

     * Receives a buffer for the transaction.

     * Receives a buffer for the transaction. The output parameter handle is

     * cloned from the internal handle. So it is safe to use directly, and it

     * should be deleted and destoyed after use.

     *

     * @param connectionId  The id of the receiving connection.

     * @param transactionId The id for the transaction.

media/codec2/hidl/1.0/utils/types.cpp

+5 −0
Original line number Diff line number Diff line
@@ -1434,6 +1434,11 @@ bool objcpy(C2BaseBlock* d, const BaseBlock& s) { 
                d->type = C2BaseBlock::GRAPHIC;

                return true;

            }

            if (cHandle) {

                // Though we got cloned handle, creating block failed.

                native_handle_close(cHandle);

                native_handle_delete(cHandle);

            }



            LOG(ERROR) << "Unknown handle type in BaseBlock::pooledBlock.";

            return false;

media/codec2/vndk/C2Buffer.cpp

+34 −46
Original line number Diff line number Diff line
@@ -413,9 +413,7 @@ std::shared_ptr<C2LinearBlock> _C2BlockFactory::CreateLinearBlock( 


    std::shared_ptr<C2LinearAllocation> alloc;

    if (C2AllocatorIon::isValid(cHandle)) {

        native_handle_t *handle = native_handle_clone(cHandle);

        if (handle) {

            c2_status_t err = sAllocator->priorLinearAllocation(handle, &alloc);

        c2_status_t err = sAllocator->priorLinearAllocation(cHandle, &alloc);

        const std::shared_ptr<C2PooledBlockPoolData> poolData =

                std::make_shared<C2PooledBlockPoolData>(data);

        if (err == C2_OK && poolData) {
@@ -425,7 +423,6 @@ std::shared_ptr<C2LinearBlock> _C2BlockFactory::CreateLinearBlock( 
            return block;

        }

    }

    }

    return nullptr;

};
@@ -674,19 +671,16 @@ public: 
        ResultStatus status = mBufferPoolManager->allocate(

                mConnectionId, params, &cHandle, &bufferPoolData);

        if (status == ResultStatus::OK) {

            native_handle_t *handle = native_handle_clone(cHandle);

            if (handle) {

            std::shared_ptr<C2LinearAllocation> alloc;

            std::shared_ptr<C2PooledBlockPoolData> poolData =

                    std::make_shared<C2PooledBlockPoolData>(bufferPoolData);

                c2_status_t err = mAllocator->priorLinearAllocation(handle, &alloc);

            c2_status_t err = mAllocator->priorLinearAllocation(cHandle, &alloc);

            if (err == C2_OK && poolData && alloc) {

                *block = _C2BlockFactory::CreateLinearBlock(alloc, poolData, 0, capacity);

                if (*block) {

                    return C2_OK;

                }

            }

            }

            return C2_NO_MEMORY;

        }

        if (status == ResultStatus::NO_MEMORY) {
@@ -710,13 +704,11 @@ public: 
        ResultStatus status = mBufferPoolManager->allocate(

                mConnectionId, params, &cHandle, &bufferPoolData);

        if (status == ResultStatus::OK) {

            native_handle_t *handle = native_handle_clone(cHandle);

            if (handle) {

            std::shared_ptr<C2GraphicAllocation> alloc;

            std::shared_ptr<C2PooledBlockPoolData> poolData =

                std::make_shared<C2PooledBlockPoolData>(bufferPoolData);

            c2_status_t err = mAllocator->priorGraphicAllocation(

                        handle, &alloc);

                    cHandle, &alloc);

            if (err == C2_OK && poolData && alloc) {

                *block = _C2BlockFactory::CreateGraphicBlock(

                        alloc, poolData, C2Rect(width, height));
@@ -724,7 +716,6 @@ public: 
                    return C2_OK;

                }

            }

            }

            return C2_NO_MEMORY;

        }

        if (status == ResultStatus::NO_MEMORY) {
@@ -1117,9 +1108,7 @@ std::shared_ptr<C2GraphicBlock> _C2BlockFactory::CreateGraphicBlock( 


    std::shared_ptr<C2GraphicAllocation> alloc;

    if (C2AllocatorGralloc::isValid(cHandle)) {

        native_handle_t *handle = native_handle_clone(cHandle);

        if (handle) {

            c2_status_t err = sAllocator->priorGraphicAllocation(handle, &alloc);

        c2_status_t err = sAllocator->priorGraphicAllocation(cHandle, &alloc);

        const std::shared_ptr<C2PooledBlockPoolData> poolData =

                std::make_shared<C2PooledBlockPoolData>(data);

        if (err == C2_OK && poolData) {
@@ -1129,7 +1118,6 @@ std::shared_ptr<C2GraphicBlock> _C2BlockFactory::CreateGraphicBlock( 
            return block;

        }

    }

    }

    return nullptr;

};