Merge changes from topic "hiddenCameraDeadlock"

* changes:
  cameraserver: Avoiding deadlocks while calling isPublicallyHiddenSecureCamera().
  Do not include hidden secure cameras in camera1: getNumberOfCameras

services/camera/libcameraservice/CameraService.cpp

@@ -253,6 +253,15 @@ void CameraService::onNewProviderRegistered() {
     enumerateProviders();
 }
 
+bool CameraService::isPublicallyHiddenSecureCamera(const String8& cameraId) {
+    auto state = getCameraState(cameraId);
+    if (state != nullptr) {
+        return state->isPublicallyHiddenSecureCamera();
+    }
+    // Hidden physical camera ids won't have CameraState
+    return mCameraProviderManager->isPublicallyHiddenSecureCamera(cameraId.c_str());
+}
+
 void CameraService::updateCameraNumAndIds() {
     Mutex::Autolock l(mServiceLock);
     mNumberOfCameras = mCameraProviderManager->getCameraCount();
@@ -268,6 +277,8 @@ void CameraService::addStates(const String8 id) {
         ALOGE("Failed to query device resource cost: %s (%d)", strerror(-res), res);
         return;
     }
+    bool isPublicallyHiddenSecureCamera =
+            mCameraProviderManager->isPublicallyHiddenSecureCamera(id.string());
     std::set<String8> conflicting;
     for (size_t i = 0; i < cost.conflictingDevices.size(); i++) {
         conflicting.emplace(String8(cost.conflictingDevices[i].c_str()));
@@ -276,7 +287,8 @@ void CameraService::addStates(const String8 id) {
     {
         Mutex::Autolock lock(mCameraStatesLock);
         mCameraStates.emplace(id, std::make_shared<CameraState>(id, cost.resourceCost,
-                                                                conflicting));
+                                                                conflicting,
+                                                                isPublicallyHiddenSecureCamera));
     }
 
     if (mFlashlight->hasFlashUnit(id)) {
@@ -514,8 +526,16 @@ Status CameraService::getCameraCharacteristics(const String16& cameraId,
                 "Camera subsystem is not available");;
     }
 
-    Status ret{};
+    if (shouldRejectHiddenCameraConnection(String8(cameraId))) {
+        ALOGW("Attempting to retrieve characteristics for system-only camera id %s, rejected",
+              String8(cameraId).string());
+        return STATUS_ERROR_FMT(ERROR_DISCONNECTED,
+                                "No camera device with ID \"%s\" currently available",
+                                String8(cameraId).string());
+
+    }
 
+    Status ret{};
     status_t res = mCameraProviderManager->getCameraCharacteristics(
             String8(cameraId).string(), cameraInfo);
     if (res != OK) {
@@ -1330,7 +1350,7 @@ bool CameraService::shouldRejectHiddenCameraConnection(const String8 & cameraId)
     // publically hidden, we should reject the connection.
     if (!hardware::IPCThreadState::self()->isServingCall() &&
             CameraThreadState::getCallingPid() != getpid() &&
-            mCameraProviderManager->isPublicallyHiddenSecureCamera(cameraId.c_str())) {
+            isPublicallyHiddenSecureCamera(cameraId)) {
         return true;
     }
     return false;
@@ -1799,16 +1819,25 @@ Status CameraService::addListenerHelper(const sp<ICameraServiceListener>& listen
     {
         Mutex::Autolock lock(mCameraStatesLock);
         for (auto& i : mCameraStates) {
-            if (!isVendorListener &&
-                mCameraProviderManager->isPublicallyHiddenSecureCamera(i.first.c_str())) {
-                ALOGV("Cannot add public listener for hidden system-only %s for pid %d",
-                      i.first.c_str(), CameraThreadState::getCallingPid());
-                continue;
-            }
             cameraStatuses->emplace_back(i.first, mapToInterface(i.second->getStatus()));
         }
     }
 
+    // Remove the camera statuses that should be hidden from the client, we do
+    // this after collecting the states in order to avoid holding
+    // mCameraStatesLock and mInterfaceLock (held in
+    // isPublicallyHiddenSecureCamera()) at the same time.
+    cameraStatuses->erase(std::remove_if(cameraStatuses->begin(), cameraStatuses->end(),
+                [this, &isVendorListener](const hardware::CameraStatus& s) {
+                    bool ret = !isVendorListener && isPublicallyHiddenSecureCamera(s.cameraId);
+                    if (ret) {
+                        ALOGV("Cannot add public listener for hidden system-only %s for pid %d",
+                                s.cameraId.c_str(), CameraThreadState::getCallingPid());
+                    }
+                    return ret;
+                }),
+                cameraStatuses->end());
+
     /*
      * Immediately signal current torch status to this listener only
      * This may be a subset of all the devices, so don't include it in the response directly
@@ -2870,8 +2899,9 @@ void CameraService::SensorPrivacyPolicy::binderDied(const wp<IBinder>& /*who*/)
 // ----------------------------------------------------------------------------
 
 CameraService::CameraState::CameraState(const String8& id, int cost,
-        const std::set<String8>& conflicting) : mId(id),
+        const std::set<String8>& conflicting, bool isHidden) : mId(id),
-        mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting) {}
+        mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting),
+        mIsPublicallyHiddenSecureCamera(isHidden) {}
 
 CameraService::CameraState::~CameraState() {}
 
@@ -2900,6 +2930,10 @@ String8 CameraService::CameraState::getId() const {
     return mId;
 }
 
+bool CameraService::CameraState::isPublicallyHiddenSecureCamera() const {
+    return mIsPublicallyHiddenSecureCamera;
+}
+
 // ----------------------------------------------------------------------------
 //                  ClientEventListener
 // ----------------------------------------------------------------------------
@@ -3235,10 +3269,10 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId,
                 cameraId.string());
         return;
     }
-
+    bool isHidden = isPublicallyHiddenSecureCamera(cameraId);
     // Update the status for this camera state, then send the onStatusChangedCallbacks to each
     // of the listeners with both the mStatusStatus and mStatusListenerLock held
-    state->updateStatus(status, cameraId, rejectSourceStates, [this]
+    state->updateStatus(status, cameraId, rejectSourceStates, [this,&isHidden]
             (const String8& cameraId, StatusInternal status) {
 
             if (status != StatusInternal::ENUMERATING) {
@@ -3260,8 +3294,7 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId,
             Mutex::Autolock lock(mStatusListenerLock);
 
             for (auto& listener : mListenerList) {
-                if (!listener.first &&
+                if (!listener.first &&  isHidden) {
-                    mCameraProviderManager->isPublicallyHiddenSecureCamera(cameraId.c_str())) {
                     ALOGV("Skipping camera discovery callback for system-only camera %s",
                           cameraId.c_str());
                     continue;

services/camera/libcameraservice/CameraService.h

@@ -471,7 +471,8 @@ private:
          * Make a new CameraState and set the ID, cost, and conflicting devices using the values
          * returned in the HAL's camera_info struct for each device.
          */
-        CameraState(const String8& id, int cost, const std::set<String8>& conflicting);
+        CameraState(const String8& id, int cost, const std::set<String8>& conflicting,
+                bool isHidden);
         virtual ~CameraState();
 
         /**
@@ -523,6 +524,11 @@ private:
          */
         String8 getId() const;
 
+        /**
+         * Return if the camera device is a publically hidden secure camera
+         */
+        bool isPublicallyHiddenSecureCamera() const;
+
     private:
         const String8 mId;
         StatusInternal mStatus; // protected by mStatusLock
@@ -530,6 +536,7 @@ private:
         std::set<String8> mConflicting;
         mutable Mutex mStatusLock;
         CameraParameters mShimParams;
+        const bool mIsPublicallyHiddenSecureCamera;
     }; // class CameraState
 
     // Observer for UID lifecycle enforcing that UIDs in idle
@@ -635,7 +642,9 @@ private:
 
     // Should an operation attempt on a cameraId be rejected, if the camera id is
     // advertised as a publically hidden secure camera, by the camera HAL ?
-    bool shouldRejectHiddenCameraConnection(const String8 & cameraId);
+    bool shouldRejectHiddenCameraConnection(const String8& cameraId);
+
+    bool isPublicallyHiddenSecureCamera(const String8& cameraId);
 
     // Single implementation shared between the various connect calls
     template<class CALLBACK, class CLIENT>

services/camera/libcameraservice/common/CameraProviderManager.cpp

@@ -97,7 +97,13 @@ int CameraProviderManager::getCameraCount() const {
     std::lock_guard<std::mutex> lock(mInterfaceMutex);
     int count = 0;
     for (auto& provider : mProviders) {
-        count += provider->mUniqueCameraIds.size();
+        for (auto& id : provider->mUniqueCameraIds) {
+            // Hidden secure camera ids are not to be exposed to camera1 api.
+            if (isPublicallyHiddenSecureCameraLocked(id)) {
+                continue;
+            }
+            count++;
+        }
     }
     return count;
 }
@@ -123,7 +129,11 @@ std::vector<std::string> CameraProviderManager::getAPI1CompatibleCameraDeviceIds
         // for each camera facing, only take the first id advertised by HAL in
         // all [logical, physical1, physical2, ...] id combos, and filter out the rest.
         filterLogicalCameraIdsLocked(providerDeviceIds);
-
+        // Hidden secure camera ids are not to be exposed to camera1 api.
+        providerDeviceIds.erase(std::remove_if(providerDeviceIds.begin(), providerDeviceIds.end(),
+                [this](const std::string& s) {
+                    return this->isPublicallyHiddenSecureCameraLocked(s);}),
+                providerDeviceIds.end());
         deviceIds.insert(deviceIds.end(), providerDeviceIds.begin(), providerDeviceIds.end());
     }
 
@@ -1035,23 +1045,42 @@ bool CameraProviderManager::isLogicalCamera(const std::string& id,
     return deviceInfo->mIsLogicalCamera;
 }
 
-bool CameraProviderManager::isPublicallyHiddenSecureCamera(const std::string& id) {
+bool CameraProviderManager::isPublicallyHiddenSecureCamera(const std::string& id) const {
     std::lock_guard<std::mutex> lock(mInterfaceMutex);
+    return isPublicallyHiddenSecureCameraLocked(id);
+}
 
+bool CameraProviderManager::isPublicallyHiddenSecureCameraLocked(const std::string& id) const {
     auto deviceInfo = findDeviceInfoLocked(id);
-    if (deviceInfo == nullptr) {
+    if (deviceInfo != nullptr) {
-        return false;
+        return deviceInfo->mIsPublicallyHiddenSecureCamera;
+    }
+    // If this is a hidden physical camera, we should return what kind of
+    // camera the enclosing logical camera is.
+    auto isHiddenAndParent = isHiddenPhysicalCameraInternal(id);
+    if (isHiddenAndParent.first) {
+        LOG_ALWAYS_FATAL_IF(id == isHiddenAndParent.second->mId,
+                "%s: hidden physical camera id %s and enclosing logical camera id %s are the same",
+                __FUNCTION__, id.c_str(), isHiddenAndParent.second->mId.c_str());
+        return isPublicallyHiddenSecureCameraLocked(isHiddenAndParent.second->mId);
     }
-    return deviceInfo->mIsPublicallyHiddenSecureCamera;
+    // Invalid camera id
+    return true;
 }
 
-bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) {
+bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) const {
+    return isHiddenPhysicalCameraInternal(cameraId).first;
+}
+
+std::pair<bool, CameraProviderManager::ProviderInfo::DeviceInfo *>
+CameraProviderManager::isHiddenPhysicalCameraInternal(const std::string& cameraId) const {
+    auto falseRet = std::make_pair(false, nullptr);
     for (auto& provider : mProviders) {
         for (auto& deviceInfo : provider->mDevices) {
             if (deviceInfo->mId == cameraId) {
                 // cameraId is found in public camera IDs advertised by the
                 // provider.
-                return false;
+                return falseRet;
             }
         }
     }
@@ -1063,7 +1092,7 @@ bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId)
             if (res != OK) {
                 ALOGE("%s: Failed to getCameraCharacteristics for id %s", __FUNCTION__,
                         deviceInfo->mId.c_str());
-                return false;
+                return falseRet;
             }
 
             std::vector<std::string> physicalIds;
@@ -1075,16 +1104,16 @@ bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId)
                     if (deviceVersion < CAMERA_DEVICE_API_VERSION_3_5) {
                         ALOGE("%s: Wrong deviceVersion %x for hiddenPhysicalCameraId %s",
                                 __FUNCTION__, deviceVersion, cameraId.c_str());
-                        return false;
+                        return falseRet;
                     } else {
-                        return true;
+                        return std::make_pair(true, deviceInfo.get());
                     }
                 }
             }
         }
     }
 
-    return false;
+    return falseRet;
 }
 
 status_t CameraProviderManager::addProviderLocked(const std::string& newProvider, bool expected) {

services/camera/libcameraservice/common/CameraProviderManager.h

@@ -269,8 +269,8 @@ public:
      */
     bool isLogicalCamera(const std::string& id, std::vector<std::string>* physicalCameraIds);
 
-    bool isPublicallyHiddenSecureCamera(const std::string& id);
+    bool isPublicallyHiddenSecureCamera(const std::string& id) const;
-    bool isHiddenPhysicalCamera(const std::string& cameraId);
+    bool isHiddenPhysicalCamera(const std::string& cameraId) const;
 
     static const float kDepthARTolerance;
 private:
@@ -591,7 +591,15 @@ private:
 
     status_t getCameraCharacteristicsLocked(const std::string &id,
             CameraMetadata* characteristics) const;
+
+    bool isPublicallyHiddenSecureCameraLocked(const std::string& id) const;
+
     void filterLogicalCameraIdsLocked(std::vector<std::string>& deviceIds) const;
+
+    bool isPublicallyHiddenSecureCameraLocked(const std::string& id);
+
+    std::pair<bool, CameraProviderManager::ProviderInfo::DeviceInfo *>
+            isHiddenPhysicalCameraInternal(const std::string& cameraId) const;
 };
 
 } // namespace android