Merge "cameraserver: Avoiding deadlocks while calling isPublicallyHiddenSecureCamera()." into qt-qpr1-dev
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index 2f88957..b20c9a4 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -256,6 +256,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();
@@ -271,6 +280,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()));
@@ -279,7 +290,8 @@
{
Mutex::Autolock lock(mCameraStatesLock);
mCameraStates.emplace(id, std::make_shared<CameraState>(id, cost.resourceCost,
- conflicting));
+ conflicting,
+ isPublicallyHiddenSecureCamera));
}
if (mFlashlight->hasFlashUnit(id)) {
@@ -517,8 +529,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) {
@@ -1333,7 +1353,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;
@@ -1807,16 +1827,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
@@ -2876,8 +2905,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() {}
@@ -2906,6 +2936,10 @@
return mId;
}
+bool CameraService::CameraState::isPublicallyHiddenSecureCamera() const {
+ return mIsPublicallyHiddenSecureCamera;
+}
+
// ----------------------------------------------------------------------------
// ClientEventListener
// ----------------------------------------------------------------------------
@@ -3241,10 +3275,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) {
@@ -3266,8 +3300,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 065157d..22842a1 100644
--- a/services/camera/libcameraservice/CameraService.h
+++ b/services/camera/libcameraservice/CameraService.h
@@ -470,7 +470,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();
/**
@@ -522,6 +523,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
@@ -529,6 +535,7 @@
std::set<String8> mConflicting;
mutable Mutex mStatusLock;
CameraParameters mShimParams;
+ const bool mIsPublicallyHiddenSecureCamera;
}; // class CameraState
// Observer for UID lifecycle enforcing that UIDs in idle
@@ -633,7 +640,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 7aa6714..fdb5657 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -1063,19 +1063,35 @@
bool CameraProviderManager::isPublicallyHiddenSecureCameraLocked(const std::string& id) const {
auto deviceInfo = findDeviceInfoLocked(id);
- if (deviceInfo == nullptr) {
- return false;
+ if (deviceInfo != nullptr) {
+ return deviceInfo->mIsPublicallyHiddenSecureCamera;
}
- 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) {
+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;
}
}
}
@@ -1087,7 +1103,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;
@@ -1099,16 +1115,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) {
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index 01eb56f..3a4655c 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -273,7 +273,7 @@
bool isLogicalCamera(const std::string& id, std::vector<std::string>* physicalCameraIds);
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:
@@ -598,6 +598,11 @@
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