From 677de6c43750b5b40557a4e78a103e74dbb906dd Mon Sep 17 00:00:00 2001 From: tdivy20 Date: Thu, 29 Aug 2024 04:41:00 +0000 Subject: [PATCH] Coverity fixes for ExternalCamera and RemoteCamera Addressed high priority coverity issues related to resource leaks, string not-null termination etc. Test done: 1. Flashed the image with fixes 2. Open AOSP camera and Multicamera app 3. Able to capture and record with both apps Tracked-On: OAM-122344 Signed-off-by: tdivy20 --- .../default/ExternalCameraDeviceSession.cpp | 4 ++++ .../default/ExternalCameraOfflineSession.cpp | 3 +++ .../default/RemoteCameraDeviceSession.cpp | 17 ++++++++++++++--- .../default/ExternalCameraProvider.cpp | 18 +++++++++++------- 4 files changed, 32 insertions(+), 10 deletions(-) diff --git a/camera/device/default/ExternalCameraDeviceSession.cpp b/camera/device/default/ExternalCameraDeviceSession.cpp index bb803a8..ee987d3 100644 --- a/camera/device/default/ExternalCameraDeviceSession.cpp +++ b/camera/device/default/ExternalCameraDeviceSession.cpp @@ -798,6 +798,7 @@ Status ExternalCameraDeviceSession::switchToOffline( native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0); handle->data[0] = buffer.acquireFence; outputBuffer.releaseFence = android::makeToAidl(handle); + native_handle_delete(handle); } } else { offlineBuffers.push_back(buffer); @@ -1781,6 +1782,7 @@ Status ExternalCameraDeviceSession::processCaptureRequestError( native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0); handle->data[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } } @@ -1827,6 +1829,7 @@ Status ExternalCameraDeviceSession::processCaptureResult(std::shared_ptrdata[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER); } else { @@ -1836,6 +1839,7 @@ Status ExternalCameraDeviceSession::processCaptureResult(std::shared_ptrdata[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } } } diff --git a/camera/device/default/ExternalCameraOfflineSession.cpp b/camera/device/default/ExternalCameraOfflineSession.cpp index da3d0ae..11b44be 100644 --- a/camera/device/default/ExternalCameraOfflineSession.cpp +++ b/camera/device/default/ExternalCameraOfflineSession.cpp @@ -111,6 +111,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptrdata[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = android::makeToAidl(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_ptrdata[0] = req->buffers[i].acquireFence; outputBuffer.releaseFence = android::makeToAidl(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 = makeToAidl(handle); + native_handle_delete(handle); } } diff --git a/camera/device/default/RemoteCameraDeviceSession.cpp b/camera/device/default/RemoteCameraDeviceSession.cpp index 3ac01ad..bedb8d5 100644 --- a/camera/device/default/RemoteCameraDeviceSession.cpp +++ b/camera/device/default/RemoteCameraDeviceSession.cpp @@ -134,7 +134,7 @@ void *RemoteDataRecvThreadFun(void *argv) ALOGI(LOG_TAG " %s: Thread is running", __FUNCTION__); struct pollfd fd; int event; - uint8_t *fBuffer = nullptr; + uint8_t *fBuffer = nullptr; RemoteCameraDeviceSession *parentHandle = (RemoteCameraDeviceSession*)argv; fd.fd = parentHandle->mFd; fd.events = POLLIN | POLLHUP; @@ -163,10 +163,15 @@ void *RemoteDataRecvThreadFun(void *argv) size_header = recv(parentHandle->mFd, (char *)&buffer_header, sizeof(camera_header_t), 0); if(buffer_header.type == CAMERA_DATA){ size_pending = (size_t)buffer_header.size; + if(fBuffer != nullptr) { + free(fBuffer); + } fBuffer = (uint8_t *)malloc(size_pending); if (fBuffer == NULL) { ALOGE(LOG_TAG "%s: buffer allocation failed: %d ", __FUNCTION__, __LINE__); + continue; } + while(true) { if(mEnqList.size() == 0) { if (parentHandle->mStopRequest == true) { @@ -216,7 +221,6 @@ void *RemoteDataRecvThreadFun(void *argv) ALOGE("updated fail to convert MJPG to I420 ret %d", res); } mDeqList.push_back(frame); - free(fBuffer); break; } } @@ -228,7 +232,10 @@ void *RemoteDataRecvThreadFun(void *argv) } } } - + if (fBuffer != nullptr) { + free(fBuffer); + fBuffer = nullptr; + } return argv; } RemoteCameraDeviceSession::RemoteCameraDeviceSession( @@ -851,6 +858,7 @@ Status RemoteCameraDeviceSession::switchToOffline( native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0); handle->data[0] = buffer.acquireFence; outputBuffer.releaseFence = android::makeToAidl(handle); + native_handle_delete(handle); } } else { offlineBuffers.push_back(buffer); @@ -1683,6 +1691,7 @@ Status RemoteCameraDeviceSession::processCaptureRequestError( native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0); handle->data[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } } @@ -1728,6 +1737,7 @@ Status RemoteCameraDeviceSession::processCaptureResult(std::shared_ptrdata[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER); } else { @@ -1737,6 +1747,7 @@ Status RemoteCameraDeviceSession::processCaptureResult(std::shared_ptrdata[0] = req->buffers[i].acquireFence; result.outputBuffers[i].releaseFence = ::android::makeToAidl(handle); + native_handle_delete(handle); } } } diff --git a/camera/provider/default/ExternalCameraProvider.cpp b/camera/provider/default/ExternalCameraProvider.cpp index dd3e0a4..34bdc7f 100644 --- a/camera/provider/default/ExternalCameraProvider.cpp +++ b/camera/provider/default/ExternalCameraProvider.cpp @@ -139,7 +139,7 @@ bool ExternalCameraProvider::configureCapabilities() { strerror(errno)); goto out; } - + free(cap_packet); cap_packet = (camera_packet_t *)malloc(cap_packet_size); if (cap_packet == NULL) { ALOGE(LOG_TAG "%s: cap camera_packet_t allocation failed: %d ", __FUNCTION__, __LINE__); @@ -351,21 +351,23 @@ void *RemoteThreadFun(void *argv) ALOGI("Waiting for connection "); int mSocketServerFd = ::socket(AF_VSOCK, SOCK_STREAM, 0); if (mSocketServerFd < 0) { - ALOGV(LOG_TAG " %s:Line:[%d] Fail to construct camera socket with error: [%s]", - __FUNCTION__, __LINE__, strerror(errno)); - return NULL; + ALOGV(LOG_TAG " %s:Line:[%d] Fail to construct camera socket with error: [%s]", + __FUNCTION__, __LINE__, strerror(errno)); + return NULL; } ret = ::bind(mSocketServerFd, (struct sockaddr *)&addr_vm, sizeof(struct sockaddr_vm)); if (ret < 0) { ALOGV(LOG_TAG " %s Failed to bind port(%d). ret: %d, %s", __func__, addr_vm.svm_port, ret, strerror(errno)); - return NULL; + close(mSocketServerFd); + return NULL; } ret = listen(mSocketServerFd, 32); if (ret < 0) { - ALOGV("%s Failed to listen on ", __FUNCTION__); - return NULL; + ALOGV("%s Failed to listen on ", __FUNCTION__); + close(mSocketServerFd); + return NULL; } socklen_t alen = sizeof(struct sockaddr_un); @@ -377,7 +379,9 @@ void *RemoteThreadFun(void *argv) ALOGE("Fail to configure "); } } + close(mSocketServerFd); } + pthread_join(thread_id, NULL); return argv; }