Implement CameraIdRemapping for API1
- For API1, there's already two Ids:
API1CameraId and a HAL camera Id,
and they might be different. We
only remap the HAL Camera Id, and leave
the API1 camera ID untouched (since
that is what the app sees). In that sense,
there was already a "remapping" from
app_camera_id to hal_camera_id in API_1, we just
remap that hal_camera_id to different
hal_camera_id_2.
In addition:
- Prevent remapping cameras for system/vendor
uids.
- Properly implement disconnect() for API1 &
API2 clients. This includes:
- Clearing binder identity before we call
disconnect(), so that the PID checks in
disconnect() impl pass.
- Not holding the mCameraIdRemappingLock
while calling disconnect(), since it can
leads to a deadlock, given a call path of
disconnect -> finishCameraOps -> updateStatus,
which also requires mCameraIdRemappingLock.
- Previously, disconnect() for API_2 clients was
likely being triggered through a side channel,
(e.g. impl in CameraDeviceClient after being
sent an ERROR_CAMERA_DEVICE), which didn't
happen for API_1 clients
Bug: 286287541
Test: Manually tested E2E for API1 and API2 apps.
Change-Id: I82b92e2c94209475f0da0b48c2993ff3b0ac7f28
Merged-In: I1e166e59e9441d8c7fe030320127abcfb373e428
diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp
index da64b8d..dd2e2d8 100644
--- a/services/camera/libcameraservice/CameraService.cpp
+++ b/services/camera/libcameraservice/CameraService.cpp
@@ -800,10 +800,9 @@
std::unique_ptr<AutoConditionLock> serviceLockWrapper =
AutoConditionLock::waitAndAcquire(mServiceLockWrapper);
- Mutex::Autolock lock(mCameraIdRemappingLock);
- // This will disconnect all existing clients for camera Ids that are being
- // remapped in cameraIdRemapping, but only if they were being used by an
- // affected packageName.
+ // Collect all existing clients for camera Ids that are being
+ // remapped in the new cameraIdRemapping, but only if they were being used by a
+ // targeted packageName.
std::vector<sp<BasicClient>> clientsToDisconnect;
std::vector<std::string> cameraIdsToUpdate;
for (const auto& [packageName, injectionMap] : cameraIdRemapping) {
@@ -814,7 +813,8 @@
if (clientDescriptor != nullptr) {
sp<BasicClient> clientSp = clientDescriptor->getValue();
if (clientSp->getPackageName() == packageName) {
- // This camera ID is being used by the affected packageName.
+ // This camera is being used by a targeted packageName and
+ // being remapped to a new camera Id. We should disconnect it.
clientsToDisconnect.push_back(clientSp);
cameraIdsToUpdate.push_back(id0);
}
@@ -822,25 +822,40 @@
}
}
- // Update mCameraIdRemapping.
- mCameraIdRemapping.clear();
- mCameraIdRemapping.insert(cameraIdRemapping.begin(), cameraIdRemapping.end());
+ for (auto& clientSp : clientsToDisconnect) {
+ // We send up ERROR_CAMERA_DEVICE so that the app attempts to reconnect
+ // automatically. Note that this itself can cause clientSp->disconnect() based on the
+ // app's response.
+ clientSp->notifyError(hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_DEVICE,
+ CaptureResultExtras{});
+ }
// Do not hold mServiceLock while disconnecting clients, but retain the condition
// blocking other clients from connecting in mServiceLockWrapper if held.
mServiceLock.unlock();
+ // Clear calling identity for disconnect() PID checks.
+ int64_t token = CameraThreadState::clearCallingIdentity();
+
// Disconnect clients.
for (auto& clientSp : clientsToDisconnect) {
- // We send up ERROR_CAMERA_DEVICE so that the app attempts to reconnect
- // automatically.
- clientSp->notifyError(hardware::camera2::ICameraDeviceCallbacks::ERROR_CAMERA_DEVICE,
- CaptureResultExtras{});
- // This also triggers the status updates
+ // This also triggers a call to updateStatus() which also reads mCameraIdRemapping
+ // and requires mCameraIdRemappingLock.
clientSp->disconnect();
}
+ // Invoke destructors (which call disconnect()) now while we don't hold the mServiceLock.
+ clientsToDisconnect.clear();
+
+ CameraThreadState::restoreCallingIdentity(token);
mServiceLock.lock();
+
+ {
+ Mutex::Autolock lock(mCameraIdRemappingLock);
+ // Update mCameraIdRemapping.
+ mCameraIdRemapping.clear();
+ mCameraIdRemapping.insert(cameraIdRemapping.begin(), cameraIdRemapping.end());
+ }
}
std::vector<std::string> CameraService::findOriginalIdsForRemappedCameraId(
@@ -859,28 +874,27 @@
return cameraIds;
}
-std::string CameraService::resolveCameraId(const std::string& inputCameraId) {
- return resolveCameraId(inputCameraId, std::string(""));
-}
-
std::string CameraService::resolveCameraId(
const std::string& inputCameraId,
+ int clientUid,
const std::string& packageName) {
std::string packageNameVal = packageName;
- if (packageName == "") {
- int clientUid = CameraThreadState::getCallingUid();
+ if (packageName.empty()) {
packageNameVal = getPackageNameFromUid(clientUid);
}
+ if (clientUid < AID_APP_START || packageNameVal.empty()) {
+ // We shouldn't remap cameras for processes with system/vendor UIDs.
+ return inputCameraId;
+ }
Mutex::Autolock lock(mCameraIdRemappingLock);
if (auto packageMapIter = mCameraIdRemapping.find(packageNameVal);
packageMapIter != mCameraIdRemapping.end()) {
- ALOGI("%s: resolveCameraId: packageName found %s",
- __FUNCTION__, packageNameVal.c_str());
auto packageMap = packageMapIter->second;
if (auto replacementIdIter = packageMap.find(inputCameraId);
replacementIdIter != packageMap.end()) {
- ALOGI("%s: resolveCameraId: inputId found %s, replacing with %s",
+ ALOGI("%s: resolveCameraId: remapping cameraId %s for %s to %s",
__FUNCTION__, inputCameraId.c_str(),
+ packageNameVal.c_str(),
replacementIdIter->second.c_str());
return replacementIdIter->second;
}
@@ -892,7 +906,10 @@
CameraInfo* cameraInfo) {
ATRACE_CALL();
Mutex::Autolock l(mServiceLock);
- std::string cameraIdStr = cameraIdIntToStrLocked(cameraId);
+ std::string unresolvedCameraId = cameraIdIntToStrLocked(cameraId);
+ std::string cameraIdStr = resolveCameraId(
+ unresolvedCameraId, CameraThreadState::getCallingUid());
+
if (shouldRejectSystemCameraConnection(cameraIdStr)) {
return STATUS_ERROR_FMT(ERROR_INVALID_OPERATION, "Unable to retrieve camera"
"characteristics for system only device %s: ", cameraIdStr.c_str());
@@ -962,7 +979,8 @@
int targetSdkVersion, bool overrideToPortrait, CameraMetadata* cameraInfo) {
ATRACE_CALL();
- const std::string cameraId = resolveCameraId(unresolvedCameraId);
+ const std::string cameraId = resolveCameraId(unresolvedCameraId,
+ CameraThreadState::getCallingUid());
if (!cameraInfo) {
ALOGE("%s: cameraInfo is NULL", __FUNCTION__);
@@ -1048,10 +1066,12 @@
return ret;
}
-Status CameraService::getTorchStrengthLevel(const std::string& cameraId,
+Status CameraService::getTorchStrengthLevel(const std::string& unresolvedCameraId,
int32_t* torchStrength) {
ATRACE_CALL();
Mutex::Autolock l(mServiceLock);
+ const std::string cameraId = resolveCameraId(
+ unresolvedCameraId, CameraThreadState::getCallingUid());
if (!mInitialized) {
ALOGE("%s: Camera HAL couldn't be initialized.", __FUNCTION__);
return STATUS_ERROR(ERROR_DISCONNECTED, "Camera HAL couldn't be initialized.");
@@ -1325,7 +1345,9 @@
return STATUS_ERROR(ERROR_ILLEGAL_ARGUMENT, "Parameters must not be null");
}
- std::string cameraIdStr = std::to_string(cameraId);
+ std::string unresolvedCameraId = std::to_string(cameraId);
+ std::string cameraIdStr = resolveCameraId(unresolvedCameraId,
+ CameraThreadState::getCallingUid());
// Check if we already have parameters
{
@@ -1838,7 +1860,10 @@
ATRACE_CALL();
Status ret = Status::ok();
- std::string cameraIdStr = cameraIdIntToStr(api1CameraId);
+ std::string unresolvedCameraId = cameraIdIntToStr(api1CameraId);
+ std::string cameraIdStr = resolveCameraId(unresolvedCameraId,
+ CameraThreadState::getCallingUid());
+
sp<Client> client = nullptr;
ret = connectHelper<ICameraClient,Client>(cameraClient, cameraIdStr, api1CameraId,
clientPackageName, /*systemNativeClient*/ false, {}, clientUid, clientPid, API_1,
@@ -1933,7 +1958,6 @@
ATRACE_CALL();
Status ret = Status::ok();
- const std::string cameraId = resolveCameraId(unresolvedCameraId, clientPackageName);
sp<CameraDeviceClient> client = nullptr;
std::string clientPackageNameAdj = clientPackageName;
int callingPid = CameraThreadState::getCallingPid();
@@ -1944,6 +1968,10 @@
clientPackageNameAdj = systemClient;
systemNativeClient = true;
}
+ const std::string cameraId = resolveCameraId(
+ unresolvedCameraId,
+ CameraThreadState::getCallingUid(),
+ clientPackageNameAdj);
if (oomScoreOffset < 0) {
std::string msg =
@@ -2434,9 +2462,8 @@
"Torch client binder in null.");
}
- const std::string cameraId = resolveCameraId(unresolvedCameraId);
int uid = CameraThreadState::getCallingUid();
-
+ const std::string cameraId = resolveCameraId(unresolvedCameraId, uid);
if (shouldRejectSystemCameraConnection(cameraId)) {
return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT, "Unable to change the strength level"
"for system only device %s: ", cameraId.c_str());
@@ -2564,8 +2591,8 @@
"Torch client Binder is null");
}
- const std::string cameraId = resolveCameraId(unresolvedCameraId);
int uid = CameraThreadState::getCallingUid();
+ const std::string cameraId = resolveCameraId(unresolvedCameraId, uid);
if (shouldRejectSystemCameraConnection(cameraId)) {
return STATUS_ERROR_FMT(ERROR_ILLEGAL_ARGUMENT, "Unable to set torch mode"
@@ -3096,15 +3123,8 @@
/*out*/ bool *isSupported) {
ATRACE_CALL();
- std::string resolvedCameraId;
- if (apiVersion == API_VERSION_2) {
- resolvedCameraId = resolveCameraId(unresolvedCameraId);
- } else { // if (apiVersion == API_VERSION_1)
- // We don't support remapping for API 1.
- // TODO(b/286287541): Also support remapping for API 1.
- resolvedCameraId = unresolvedCameraId;
- }
- const std::string cameraId = resolvedCameraId;
+ const std::string cameraId = resolveCameraId(
+ unresolvedCameraId, CameraThreadState::getCallingUid());
ALOGV("%s: for camera ID = %s", __FUNCTION__, cameraId.c_str());
@@ -3168,7 +3188,8 @@
/*out*/ bool *isSupported) {
ATRACE_CALL();
- const std::string cameraId = resolveCameraId(unresolvedCameraId);
+ const std::string cameraId = resolveCameraId(unresolvedCameraId,
+ CameraThreadState::getCallingUid());
ALOGV("%s: for camera ID = %s", __FUNCTION__, cameraId.c_str());
*isSupported = mCameraProviderManager->isHiddenPhysicalCamera(cameraId);