From b3b02babbb85747e98b79a4b8744b0ca51d7c40c Mon Sep 17 00:00:00 2001
From: Avichal Rakesh <arakesh@google.com>
Date: Fri, 12 Jan 2024 17:28:28 -0800
Subject: [PATCH] ExternalCameraHAL: Prevent memory leak when manipulating
 native_handle

native_handle objects created by makeFromAidl need to be deleted
with native_handle_delete. Not doing so leads to a memory leak
every time makeFromAidl is called.

This CL ensures that native_handle_delete is called on the
return value of makeFromAidl wherever it is used.

Bug: 305638723
Test: n/a. No functional change.
Merged-in: Ia99ba6e3abbdf7dec75383450a60c944b92a9c74
Change-Id: Ia99ba6e3abbdf7dec75383450a60c944b92a9c74
---
 .../default/ExternalCameraDeviceSession.cpp   | 19 ++++++++++++-------
 .../default/ExternalCameraOfflineSession.cpp  |  3 +++
 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/camera/device/default/ExternalCameraDeviceSession.cpp b/camera/device/default/ExternalCameraDeviceSession.cpp
index 126b782112..a16dd7fd83 100644
--- a/camera/device/default/ExternalCameraDeviceSession.cpp
+++ b/camera/device/default/ExternalCameraDeviceSession.cpp
@@ -1391,12 +1391,14 @@ Status ExternalCameraDeviceSession::importRequestLockedImpl(
 
     // All buffers are imported. Now validate output buffer acquire fences
     for (size_t i = 0; i < numOutputBufs; i++) {
-        if (!sHandleImporter.importFence(
-                    ::android::makeFromAidl(request.outputBuffers[i].acquireFence), allFences[i])) {
+        native_handle_t* h = ::android::makeFromAidl(request.outputBuffers[i].acquireFence);
+        if (!sHandleImporter.importFence(h, allFences[i])) {
             ALOGE("%s: output buffer %zu acquire fence is invalid", __FUNCTION__, i);
             cleanupInflightFences(allFences, i);
+            native_handle_delete(h);
             return Status::INTERNAL_ERROR;
         }
+        native_handle_delete(h);
     }
     return Status::OK;
 }
@@ -2094,9 +2096,10 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() {
                     // TODO: create a batch import API so we don't need to lock/unlock mCbsLock
                     // repeatedly?
                     lk.unlock();
-                    Status s =
-                            parent->importBuffer(streamId, hBuf.bufferId, makeFromAidl(hBuf.buffer),
-                                                 /*out*/ &mBufferReqs[i].bufPtr);
+                    native_handle_t* h = makeFromAidl(hBuf.buffer);
+                    Status s = parent->importBuffer(streamId, hBuf.bufferId, h,
+                                                    /*out*/ &mBufferReqs[i].bufPtr);
+                    native_handle_delete(h);
                     lk.lock();
 
                     if (s != Status::OK) {
@@ -2104,12 +2107,14 @@ bool ExternalCameraDeviceSession::BufferRequestThread::threadLoop() {
                         cleanupInflightFences(importedFences, i - 1);
                         return false;
                     }
-                    if (!sHandleImporter.importFence(makeFromAidl(hBuf.acquireFence),
-                                                     mBufferReqs[i].acquireFence)) {
+                    h = makeFromAidl(hBuf.acquireFence);
+                    if (!sHandleImporter.importFence(h, mBufferReqs[i].acquireFence)) {
                         ALOGE("%s: stream %d import fence failed!", __FUNCTION__, streamId);
                         cleanupInflightFences(importedFences, i - 1);
+                        native_handle_delete(h);
                         return false;
                     }
+                    native_handle_delete(h);
                     importedFences[i] = mBufferReqs[i].acquireFence;
                 } break;
                 default:
diff --git a/camera/device/default/ExternalCameraOfflineSession.cpp b/camera/device/default/ExternalCameraOfflineSession.cpp
index 536fa47a61..2d4e2e0234 100644
--- a/camera/device/default/ExternalCameraOfflineSession.cpp
+++ b/camera/device/default/ExternalCameraOfflineSession.cpp
@@ -111,6 +111,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq
                 native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
                 handle->data[0] = req->buffers[i].acquireFence;
                 result.outputBuffers[i].releaseFence = android::dupToAidl(handle);
+                native_handle_delete(handle);
             }
             notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);
         } else {
@@ -120,6 +121,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq
                 native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
                 handle->data[0] = req->buffers[i].acquireFence;
                 outputBuffer.releaseFence = android::dupToAidl(handle);
+                native_handle_delete(handle);
             }
         }
     }
@@ -248,6 +250,7 @@ Status ExternalCameraOfflineSession::processCaptureRequestError(
             native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);
             handle->data[0] = req->buffers[i].acquireFence;
             outputBuffer.releaseFence = dupToAidl(handle);
+            native_handle_delete(handle);
         }
     }