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
Merged-In: I4ac142fcb7e3ecdb3fb02792f516b343d71bfc38
Change-Id: I4ac142fcb7e3ecdb3fb02792f516b343d71bfc38
(cherry picked from commit dc5cb62ff4)

media/bufferpool/2.0/ClientManager.cpp

@@ -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

@@ -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

@@ -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

@@ -413,17 +413,14 @@ 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);
-            const std::shared_ptr<C2PooledBlockPoolData> poolData =
-                    std::make_shared<C2PooledBlockPoolData>(data);
-            if (err == C2_OK && poolData) {
-                // TODO: config params?
-                std::shared_ptr<C2LinearBlock> block =
-                        _C2BlockFactory::CreateLinearBlock(alloc, poolData);
-                return block;
-            }
+        c2_status_t err = sAllocator->priorLinearAllocation(cHandle, &alloc);
+        const std::shared_ptr<C2PooledBlockPoolData> poolData =
+                std::make_shared<C2PooledBlockPoolData>(data);
+        if (err == C2_OK && poolData) {
+            // TODO: config params?
+            std::shared_ptr<C2LinearBlock> block =
+                    _C2BlockFactory::CreateLinearBlock(alloc, poolData);
+            return block;
         }
     }
     return nullptr;
@@ -674,17 +671,14 @@ 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);
-                if (err == C2_OK && poolData && alloc) {
-                    *block = _C2BlockFactory::CreateLinearBlock(alloc, poolData, 0, capacity);
-                    if (*block) {
-                        return C2_OK;
-                    }
+            std::shared_ptr<C2LinearAllocation> alloc;
+            std::shared_ptr<C2PooledBlockPoolData> poolData =
+                    std::make_shared<C2PooledBlockPoolData>(bufferPoolData);
+            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;
@@ -710,19 +704,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<C2GraphicAllocation> alloc;
-                std::shared_ptr<C2PooledBlockPoolData> poolData =
-                    std::make_shared<C2PooledBlockPoolData>(bufferPoolData);
-                c2_status_t err = mAllocator->priorGraphicAllocation(
-                        handle, &alloc);
-                if (err == C2_OK && poolData && alloc) {
-                    *block = _C2BlockFactory::CreateGraphicBlock(
-                            alloc, poolData, C2Rect(width, height));
-                    if (*block) {
-                        return C2_OK;
-                    }
+            std::shared_ptr<C2GraphicAllocation> alloc;
+            std::shared_ptr<C2PooledBlockPoolData> poolData =
+                std::make_shared<C2PooledBlockPoolData>(bufferPoolData);
+            c2_status_t err = mAllocator->priorGraphicAllocation(
+                    cHandle, &alloc);
+            if (err == C2_OK && poolData && alloc) {
+                *block = _C2BlockFactory::CreateGraphicBlock(
+                        alloc, poolData, C2Rect(width, height));
+                if (*block) {
+                    return C2_OK;
                 }
             }
             return C2_NO_MEMORY;
@@ -1117,17 +1108,14 @@ 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);
-            const std::shared_ptr<C2PooledBlockPoolData> poolData =
-                    std::make_shared<C2PooledBlockPoolData>(data);
-            if (err == C2_OK && poolData) {
-                // TODO: config setup?
-                std::shared_ptr<C2GraphicBlock> block =
-                        _C2BlockFactory::CreateGraphicBlock(alloc, poolData);
-                return block;
-            }
+        c2_status_t err = sAllocator->priorGraphicAllocation(cHandle, &alloc);
+        const std::shared_ptr<C2PooledBlockPoolData> poolData =
+                std::make_shared<C2PooledBlockPoolData>(data);
+        if (err == C2_OK && poolData) {
+            // TODO: config setup?
+            std::shared_ptr<C2GraphicBlock> block =
+                    _C2BlockFactory::CreateGraphicBlock(alloc, poolData);
+            return block;
         }
     }
     return nullptr;