Camera:[email protected]: Added mutex lock for mMemoryMap.

Issue: Camera provider crashing due __next_prime overflow while accessing mMemoryMap. Solution: Added mMemoryMapLock for securing mMemoryMap access from multiple threads. Change-Id: I6caa0639a11ff458775a896198bc5a964a5b7a35 CRs-fixed:2068346
repopick · Nov 26, 2017 · a19b8d9 · a19b8d9
1 parent efbac9b
commit a19b8d9
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 5 deletions.
diff --git a/camera/device/1.0-legacy/CameraDevice.cpp b/camera/device/1.0-legacy/CameraDevice.cpp
@@ -380,10 +380,13 @@ camera_memory_t* CameraDevice::sGetMemory(int fd, size_t buf_size, uint_t num_bu
     hidl_handle hidlHandle = mem->mHidlHandle;
     MemoryId id = object->mDeviceCallback->registerMemory(hidlHandle, buf_size, num_bufs);
     mem->handle.mId = id;
-    if (object->mMemoryMap.count(id) != 0) {
-        ALOGE("%s: duplicate MemoryId %d returned by client!", __FUNCTION__, id);
+    {
+        Mutex::Autolock _l(object->mMemoryMapLock);
+        if (object->mMemoryMap.count(id) != 0) {
+            ALOGE("%s: duplicate MemoryId %d returned by client!", __FUNCTION__, id);
+        }
+        object->mMemoryMap[id] = mem;
     }
-    object->mMemoryMap[id] = mem;
     mem->handle.mDevice = object;
     return &mem->handle;
 }
@@ -401,7 +404,10 @@ void CameraDevice::sPutMemory(camera_memory_t *data) {
         ALOGE("%s: camera HAL return memory while camera is not opened!", __FUNCTION__);
     }
     device->mDeviceCallback->unregisterMemory(mem->handle.mId);
-    device->mMemoryMap.erase(mem->handle.mId);
+    {
+        Mutex::Autolock _l(device->mMemoryMapLock);
+        device->mMemoryMap.erase(mem->handle.mId);
+    }
     mem->decStrong(mem);
 }
 
@@ -830,7 +836,17 @@ void CameraDevice::releaseRecordingFrameLocked(
         return;
     }
     if (mDevice->ops->release_recording_frame) {
-        CameraHeapMemory* camMemory = mMemoryMap.at(memId);
+        CameraHeapMemory* camMemory;
+        {
+            Mutex::Autolock _l(mMemoryMapLock);
+            auto it = mMemoryMap.find(memId);
+            if (it == mMemoryMap.end() || it->second == nullptr) {
+                ALOGE("%s unknown memoryId %d", __FUNCTION__, memId);
+                return;
+            }
+            camMemory = it->second;
+        }
+
         if (bufferIndex >= camMemory->mNumBufs) {
             ALOGE("%s: bufferIndex %d exceeds number of buffers %d",
                     __FUNCTION__, bufferIndex, camMemory->mNumBufs);

diff --git a/camera/device/1.0-legacy/CameraDevice_1_0.h b/camera/device/1.0-legacy/CameraDevice_1_0.h
@@ -165,6 +165,8 @@ struct CameraDevice : public ICameraDevice {
 
     sp<ICameraDeviceCallback> mDeviceCallback = nullptr;
 
+    mutable Mutex mMemoryMapLock; // gating access to mMemoryMap
+                                 // must not hold mLock after this lock is acquired
     std::unordered_map<MemoryId, CameraHeapMemory*> mMemoryMap;
 
     bool mMetadataMode = false;