Merge changes from topic "hiddenCameraDeadlock"
am: c52243f298
Change-Id: Iced231eeaf55af13dfff514f21ce3398cdbd6a0c
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index a4868bf..c70513c 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -253,6 +253,15 @@
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 @@
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 @@
{
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 @@
"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 @@
// 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;
@@ -1804,16 +1824,25 @@
{
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
@@ -2871,8 +2900,9 @@
// ----------------------------------------------------------------------------
CameraService::CameraState::CameraState(const String8& id, int cost,
- const std::set<String8>& conflicting) : mId(id),
- mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting) {}
+ const std::set<String8>& conflicting, bool isHidden) : mId(id),
+ mStatus(StatusInternal::NOT_PRESENT), mCost(cost), mConflicting(conflicting),
+ mIsPublicallyHiddenSecureCamera(isHidden) {}
CameraService::CameraState::~CameraState() {}
@@ -2901,6 +2931,10 @@
return mId;
}
+bool CameraService::CameraState::isPublicallyHiddenSecureCamera() const {
+ return mIsPublicallyHiddenSecureCamera;
+}
+
// ----------------------------------------------------------------------------
// ClientEventListener
// ----------------------------------------------------------------------------
@@ -3236,10 +3270,10 @@
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) {
@@ -3261,8 +3295,7 @@
Mutex::Autolock lock(mStatusListenerLock);
for (auto& listener : mListenerList) {
- if (!listener.first &&
- mCameraProviderManager->isPublicallyHiddenSecureCamera(cameraId.c_str())) {
+ if (!listener.first && isHidden) {
ALOGV("Skipping camera discovery callback for system-only camera %s",
cameraId.c_str());
continue;
diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h
index cf93a41..8bb78cd 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -471,7 +471,8 @@
* 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 @@
*/
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 @@
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 @@
// 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>
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index 98f9328..cb2c324 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -97,7 +97,13 @@
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 @@
// 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 @@
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);
-
- auto deviceInfo = findDeviceInfoLocked(id);
- if (deviceInfo == nullptr) {
- return false;
- }
- return deviceInfo->mIsPublicallyHiddenSecureCamera;
+ return isPublicallyHiddenSecureCameraLocked(id);
}
-bool CameraProviderManager::isHiddenPhysicalCamera(const std::string& cameraId) {
+bool CameraProviderManager::isPublicallyHiddenSecureCameraLocked(const std::string& id) const {
+ auto deviceInfo = findDeviceInfoLocked(id);
+ if (deviceInfo != nullptr) {
+ 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);
+ }
+ // Invalid camera id
+ return true;
+}
+
+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 @@
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 @@
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) {
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index a42fb4d..cd283b3 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -269,8 +269,8 @@
*/
bool isLogicalCamera(const std::string& id, std::vector<std::string>* physicalCameraIds);
- bool isPublicallyHiddenSecureCamera(const std::string& id);
- bool isHiddenPhysicalCamera(const std::string& cameraId);
+ bool isPublicallyHiddenSecureCamera(const std::string& id) const;
+ bool isHiddenPhysicalCamera(const std::string& cameraId) const;
static const float kDepthARTolerance;
private:
@@ -591,7 +591,15 @@
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