From 9e648f6d57901b8d2c6d595d08d0cb55d87981e4 Mon Sep 17 00:00:00 2001 From: "Philip P. Moltmann" Date: Mon, 4 Nov 2019 12:52:45 -0800 Subject: [PATCH] Pipe through featureId from context to Camera code Bug: 136595429 Test: atest CtsAppOpsTestCases (now including two new test cases that open a camera with a null and a non-null feature) Change-Id: Idfb8f8049dff536525d4f081151c79d980d76c69 --- .../aidl/android/hardware/ICameraService.aidl | 1 + camera/ndk/impl/ACameraManager.cpp | 2 +- camera/tests/CameraBinderTests.cpp | 6 +- .../camera/libcameraservice/CameraService.cpp | 59 +++++++++++-------- .../camera/libcameraservice/CameraService.h | 17 ++++-- .../libcameraservice/api1/Camera2Client.cpp | 3 +- .../libcameraservice/api1/Camera2Client.h | 1 + .../libcameraservice/api1/CameraClient.cpp | 4 +- .../libcameraservice/api1/CameraClient.h | 1 + .../api2/CameraDeviceClient.cpp | 5 +- .../api2/CameraDeviceClient.h | 2 + .../common/Camera2ClientBase.cpp | 3 +- .../common/Camera2ClientBase.h | 1 + .../hidl/HidlCameraService.cpp | 2 +- 14 files changed, 69 insertions(+), 38 deletions(-) diff --git a/camera/aidl/android/hardware/ICameraService.aidl b/camera/aidl/android/hardware/ICameraService.aidl index 3e8992aaaf..62dbb5e29a 100644 --- a/camera/aidl/android/hardware/ICameraService.aidl +++ b/camera/aidl/android/hardware/ICameraService.aidl @@ -87,6 +87,7 @@ interface ICameraService ICameraDeviceUser connectDevice(ICameraDeviceCallbacks callbacks, String cameraId, String opPackageName, + @nullable String featureId, int clientUid); /** diff --git a/camera/ndk/impl/ACameraManager.cpp b/camera/ndk/impl/ACameraManager.cpp index 9d40fd7144..8eb030a03d 100644 --- a/camera/ndk/impl/ACameraManager.cpp +++ b/camera/ndk/impl/ACameraManager.cpp @@ -517,7 +517,7 @@ ACameraManager::openCamera( // No way to get package name from native. // Send a zero length package name and let camera service figure it out from UID binder::Status serviceRet = cs->connectDevice( - callbacks, String16(cameraId), String16(""), + callbacks, String16(cameraId), String16(""), std::unique_ptr(), hardware::ICameraService::USE_CALLING_UID, /*out*/&deviceRemote); if (!serviceRet.isOk()) { diff --git a/camera/tests/CameraBinderTests.cpp b/camera/tests/CameraBinderTests.cpp index f07a1e6908..9a18b10222 100644 --- a/camera/tests/CameraBinderTests.cpp +++ b/camera/tests/CameraBinderTests.cpp @@ -361,7 +361,8 @@ TEST(CameraServiceBinderTest, CheckBinderCameraService) { sp callbacks(new TestCameraDeviceCallbacks()); sp device; res = service->connectDevice(callbacks, cameraId, String16("meeeeeeeee!"), - hardware::ICameraService::USE_CALLING_UID, /*out*/&device); + std::unique_ptr(), hardware::ICameraService::USE_CALLING_UID, + /*out*/&device); EXPECT_TRUE(res.isOk()) << res; ASSERT_NE(nullptr, device.get()); device->disconnect(); @@ -403,7 +404,8 @@ protected: { SCOPED_TRACE("openNewDevice"); binder::Status res = service->connectDevice(callbacks, deviceId, String16("meeeeeeeee!"), - hardware::ICameraService::USE_CALLING_UID, /*out*/&device); + std::unique_ptr(), hardware::ICameraService::USE_CALLING_UID, + /*out*/&device); EXPECT_TRUE(res.isOk()) << res; } auto p = std::make_pair(callbacks, device); diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index c10adbb135..b704573a32 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -732,9 +732,10 @@ Status CameraService::filterGetInfoErrorCode(status_t err) { } Status CameraService::makeClient(const sp& cameraService, - const sp& cameraCb, const String16& packageName, const String8& cameraId, - int api1CameraId, int facing, int clientPid, uid_t clientUid, int servicePid, - int halVersion, int deviceVersion, apiLevel effectiveApiLevel, + const sp& cameraCb, const String16& packageName, + const std::unique_ptr& featureId, const String8& cameraId, int api1CameraId, + int facing, int clientPid, uid_t clientUid, int servicePid, int halVersion, + int deviceVersion, apiLevel effectiveApiLevel, /*out*/sp* client) { if (halVersion < 0 || halVersion == deviceVersion) { @@ -744,7 +745,7 @@ Status CameraService::makeClient(const sp& cameraService, case CAMERA_DEVICE_API_VERSION_1_0: if (effectiveApiLevel == API_1) { // Camera1 API route sp tmp = static_cast(cameraCb.get()); - *client = new CameraClient(cameraService, tmp, packageName, + *client = new CameraClient(cameraService, tmp, packageName, featureId, api1CameraId, facing, clientPid, clientUid, getpid()); } else { // Camera2 API route @@ -762,15 +763,15 @@ Status CameraService::makeClient(const sp& cameraService, case CAMERA_DEVICE_API_VERSION_3_5: if (effectiveApiLevel == API_1) { // Camera1 API route sp tmp = static_cast(cameraCb.get()); - *client = new Camera2Client(cameraService, tmp, packageName, + *client = new Camera2Client(cameraService, tmp, packageName, featureId, cameraId, api1CameraId, facing, clientPid, clientUid, servicePid); } else { // Camera2 API route sp tmp = static_cast(cameraCb.get()); - *client = new CameraDeviceClient(cameraService, tmp, packageName, cameraId, - facing, clientPid, clientUid, servicePid); + *client = new CameraDeviceClient(cameraService, tmp, packageName, featureId, + cameraId, facing, clientPid, clientUid, servicePid); } break; default: @@ -787,7 +788,7 @@ Status CameraService::makeClient(const sp& cameraService, halVersion == CAMERA_DEVICE_API_VERSION_1_0) { // Only support higher HAL version device opened as HAL1.0 device. sp tmp = static_cast(cameraCb.get()); - *client = new CameraClient(cameraService, tmp, packageName, + *client = new CameraClient(cameraService, tmp, packageName, featureId, api1CameraId, facing, clientPid, clientUid, servicePid); } else { @@ -887,7 +888,7 @@ Status CameraService::initializeShimMetadata(int cameraId) { if (!(ret = connectHelper( sp{nullptr}, id, cameraId, static_cast(CAMERA_HAL_API_VERSION_UNSPECIFIED), - internalPackageName, uid, USE_CALLING_PID, + internalPackageName, std::unique_ptr(), uid, USE_CALLING_PID, API_1, /*shimUpdateOnly*/ true, /*out*/ tmp) ).isOk()) { ALOGE("%s: Error initializing shim metadata: %s", __FUNCTION__, ret.toString8().string()); @@ -1400,8 +1401,8 @@ Status CameraService::connect( String8 id = cameraIdIntToStr(api1CameraId); sp client = nullptr; ret = connectHelper(cameraClient, id, api1CameraId, - CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageName, clientUid, clientPid, API_1, - /*shimUpdateOnly*/ false, /*out*/client); + CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageName, std::unique_ptr(), + clientUid, clientPid, API_1, /*shimUpdateOnly*/ false, /*out*/client); if(!ret.isOk()) { logRejected(id, CameraThreadState::getCallingPid(), String8(clientPackageName), @@ -1427,8 +1428,8 @@ Status CameraService::connectLegacy( Status ret = Status::ok(); sp client = nullptr; ret = connectHelper(cameraClient, id, api1CameraId, halVersion, - clientPackageName, clientUid, USE_CALLING_PID, API_1, /*shimUpdateOnly*/ false, - /*out*/client); + clientPackageName, std::unique_ptr(), clientUid, USE_CALLING_PID, API_1, + /*shimUpdateOnly*/ false, /*out*/client); if(!ret.isOk()) { logRejected(id, CameraThreadState::getCallingPid(), String8(clientPackageName), @@ -1502,6 +1503,7 @@ Status CameraService::connectDevice( const sp& cameraCb, const String16& cameraId, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, int clientUid, /*out*/ sp* device) { @@ -1511,6 +1513,7 @@ Status CameraService::connectDevice( String8 id = String8(cameraId); sp client = nullptr; String16 clientPackageNameAdj = clientPackageName; + if (hardware::IPCThreadState::self()->isServingCall()) { std::string vendorClient = StringPrintf("vendor.client.pid<%d>", CameraThreadState::getCallingPid()); @@ -1518,7 +1521,7 @@ Status CameraService::connectDevice( } ret = connectHelper(cameraCb, id, /*api1CameraId*/-1, - CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageNameAdj, + CAMERA_HAL_API_VERSION_UNSPECIFIED, clientPackageNameAdj, clientFeatureId, clientUid, USE_CALLING_PID, API_2, /*shimUpdateOnly*/ false, /*out*/client); if(!ret.isOk()) { @@ -1533,8 +1536,9 @@ Status CameraService::connectDevice( template Status CameraService::connectHelper(const sp& cameraCb, const String8& cameraId, - int api1CameraId, int halVersion, const String16& clientPackageName, int clientUid, - int clientPid, apiLevel effectiveApiLevel, bool shimUpdateOnly, + int api1CameraId, int halVersion, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, int clientUid, int clientPid, + apiLevel effectiveApiLevel, bool shimUpdateOnly, /*out*/sp& device) { binder::Status ret = binder::Status::ok(); @@ -1617,7 +1621,7 @@ Status CameraService::connectHelper(const sp& cameraCb, const String8& } sp tmp = nullptr; - if(!(ret = makeClient(this, cameraCb, clientPackageName, + if(!(ret = makeClient(this, cameraCb, clientPackageName, clientFeatureId, cameraId, api1CameraId, facing, clientPid, clientUid, getpid(), halVersion, deviceVersion, effectiveApiLevel, @@ -2459,13 +2463,14 @@ void CameraService::playSound(sound_kind kind) { CameraService::Client::Client(const sp& cameraService, const sp& cameraClient, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraIdStr, int api1CameraId, int cameraFacing, int clientPid, uid_t clientUid, int servicePid) : CameraService::BasicClient(cameraService, IInterface::asBinder(cameraClient), - clientPackageName, + clientPackageName, clientFeatureId, cameraIdStr, cameraFacing, clientPid, clientUid, servicePid), @@ -2495,17 +2500,24 @@ sp CameraService::BasicClient::BasicClient::sCameraService; CameraService::BasicClient::BasicClient(const sp& cameraService, const sp& remoteCallback, - const String16& clientPackageName, + const String16& clientPackageName, const std::unique_ptr& clientFeatureId, const String8& cameraIdStr, int cameraFacing, int clientPid, uid_t clientUid, int servicePid): mCameraIdStr(cameraIdStr), mCameraFacing(cameraFacing), - mClientPackageName(clientPackageName), mClientPid(clientPid), mClientUid(clientUid), + mClientPackageName(clientPackageName), + mClientPid(clientPid), mClientUid(clientUid), mServicePid(servicePid), mDisconnected(false), mAudioRestriction(hardware::camera2::ICameraDeviceUser::AUDIO_RESTRICTION_NONE), mRemoteBinder(remoteCallback) { + if (clientFeatureId) { + mClientFeatureId = std::unique_ptr(new String16(*clientFeatureId)); + } else { + mClientFeatureId = std::unique_ptr(); + } + if (sCameraService == nullptr) { sCameraService = cameraService; } @@ -2649,8 +2661,9 @@ status_t CameraService::BasicClient::startCameraOps() { int32_t res; mAppOpsManager->startWatchingMode(AppOpsManager::OP_CAMERA, mClientPackageName, mOpsCallback); - res = mAppOpsManager->startOpNoThrow(AppOpsManager::OP_CAMERA, - mClientUid, mClientPackageName, /*startIfModeDefault*/ false); + res = mAppOpsManager->startOpNoThrow(AppOpsManager::OP_CAMERA, mClientUid, + mClientPackageName, /*startIfModeDefault*/ false, mClientFeatureId, + String16("start camera ") + String16(mCameraIdStr)); if (res == AppOpsManager::MODE_ERRORED) { ALOGI("Camera %s: Access for \"%s\" has been revoked", @@ -2692,7 +2705,7 @@ status_t CameraService::BasicClient::finishCameraOps() { // Notify app ops that the camera is available again if (mAppOpsManager != nullptr) { mAppOpsManager->finishOp(AppOpsManager::OP_CAMERA, mClientUid, - mClientPackageName); + mClientPackageName, mClientFeatureId); mOpsActive = false; } // This function is called when a client disconnects. This should diff --git a/services/camera/libcameraservice/CameraService.h b/services/camera/libcameraservice/CameraService.h index 829a3ee045..1f40fc31d4 100644 --- a/services/camera/libcameraservice/CameraService.h +++ b/services/camera/libcameraservice/CameraService.h @@ -134,7 +134,8 @@ public: virtual binder::Status connectDevice( const sp& cameraCb, const String16& cameraId, - const String16& clientPackageName, int32_t clientUid, + const String16& clientPackageName, const std::unique_ptr& clientFeatureId, + int32_t clientUid, /*out*/ sp* device); @@ -275,6 +276,7 @@ public: BasicClient(const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraIdStr, int cameraFacing, int clientPid, @@ -294,6 +296,7 @@ public: const String8 mCameraIdStr; const int mCameraFacing; String16 mClientPackageName; + std::unique_ptr mClientFeatureId; pid_t mClientPid; const uid_t mClientUid; const pid_t mServicePid; @@ -365,6 +368,7 @@ public: Client(const sp& cameraService, const sp& cameraClient, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraIdStr, int api1CameraId, int cameraFacing, @@ -688,8 +692,8 @@ private: template binder::Status connectHelper(const sp& cameraCb, const String8& cameraId, int api1CameraId, int halVersion, const String16& clientPackageName, - int clientUid, int clientPid, apiLevel effectiveApiLevel, bool shimUpdateOnly, - /*out*/sp& device); + const std::unique_ptr& clientFeatureId, int clientUid, int clientPid, + apiLevel effectiveApiLevel, bool shimUpdateOnly, /*out*/sp& device); // Lock guarding camera service state Mutex mServiceLock; @@ -985,9 +989,10 @@ private: static String8 getFormattedCurrentTime(); static binder::Status makeClient(const sp& cameraService, - const sp& cameraCb, const String16& packageName, const String8& cameraId, - int api1CameraId, int facing, int clientPid, uid_t clientUid, int servicePid, - int halVersion, int deviceVersion, apiLevel effectiveApiLevel, + const sp& cameraCb, const String16& packageName, + const std::unique_ptr& featureId, const String8& cameraId, int api1CameraId, + int facing, int clientPid, uid_t clientUid, int servicePid, int halVersion, + int deviceVersion, apiLevel effectiveApiLevel, /*out*/sp* client); status_t checkCameraAccess(const String16& opPackageName); diff --git a/services/camera/libcameraservice/api1/Camera2Client.cpp b/services/camera/libcameraservice/api1/Camera2Client.cpp index c27388103d..5dbbc0bf38 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.cpp +++ b/services/camera/libcameraservice/api1/Camera2Client.cpp @@ -50,13 +50,14 @@ using namespace camera2; Camera2Client::Camera2Client(const sp& cameraService, const sp& cameraClient, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraDeviceId, int api1CameraId, int cameraFacing, int clientPid, uid_t clientUid, int servicePid): - Camera2ClientBase(cameraService, cameraClient, clientPackageName, + Camera2ClientBase(cameraService, cameraClient, clientPackageName, clientFeatureId, cameraDeviceId, api1CameraId, cameraFacing, clientPid, clientUid, servicePid), mParameters(api1CameraId, cameraFacing) diff --git a/services/camera/libcameraservice/api1/Camera2Client.h b/services/camera/libcameraservice/api1/Camera2Client.h index 8a17b17399..8034ab46b5 100644 --- a/services/camera/libcameraservice/api1/Camera2Client.h +++ b/services/camera/libcameraservice/api1/Camera2Client.h @@ -93,6 +93,7 @@ public: Camera2Client(const sp& cameraService, const sp& cameraClient, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraDeviceId, int api1CameraId, int cameraFacing, diff --git a/services/camera/libcameraservice/api1/CameraClient.cpp b/services/camera/libcameraservice/api1/CameraClient.cpp index 388a5dc849..83da880dac 100644 --- a/services/camera/libcameraservice/api1/CameraClient.cpp +++ b/services/camera/libcameraservice/api1/CameraClient.cpp @@ -34,11 +34,11 @@ namespace android { CameraClient::CameraClient(const sp& cameraService, const sp& cameraClient, - const String16& clientPackageName, + const String16& clientPackageName, const std::unique_ptr& clientFeatureId, int cameraId, int cameraFacing, int clientPid, int clientUid, int servicePid): - Client(cameraService, cameraClient, clientPackageName, + Client(cameraService, cameraClient, clientPackageName, clientFeatureId, String8::format("%d", cameraId), cameraId, cameraFacing, clientPid, clientUid, servicePid) { diff --git a/services/camera/libcameraservice/api1/CameraClient.h b/services/camera/libcameraservice/api1/CameraClient.h index b26b61292c..501ad1810d 100644 --- a/services/camera/libcameraservice/api1/CameraClient.h +++ b/services/camera/libcameraservice/api1/CameraClient.h @@ -66,6 +66,7 @@ public: CameraClient(const sp& cameraService, const sp& cameraClient, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, int cameraId, int cameraFacing, int clientPid, diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp index 08fb1535a6..27bebde10c 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.cpp +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.cpp @@ -54,6 +54,7 @@ CameraDeviceClientBase::CameraDeviceClientBase( const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int api1CameraId, int cameraFacing, @@ -63,6 +64,7 @@ CameraDeviceClientBase::CameraDeviceClientBase( BasicClient(cameraService, IInterface::asBinder(remoteCallback), clientPackageName, + clientFeatureId, cameraId, cameraFacing, clientPid, @@ -78,12 +80,13 @@ CameraDeviceClientBase::CameraDeviceClientBase( CameraDeviceClient::CameraDeviceClient(const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int cameraFacing, int clientPid, uid_t clientUid, int servicePid) : - Camera2ClientBase(cameraService, remoteCallback, clientPackageName, + Camera2ClientBase(cameraService, remoteCallback, clientPackageName, clientFeatureId, cameraId, /*API1 camera ID*/ -1, cameraFacing, clientPid, clientUid, servicePid), mInputStream(), diff --git a/services/camera/libcameraservice/api2/CameraDeviceClient.h b/services/camera/libcameraservice/api2/CameraDeviceClient.h index fe250103f1..7efc5abe18 100644 --- a/services/camera/libcameraservice/api2/CameraDeviceClient.h +++ b/services/camera/libcameraservice/api2/CameraDeviceClient.h @@ -47,6 +47,7 @@ protected: CameraDeviceClientBase(const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int api1CameraId, int cameraFacing, @@ -163,6 +164,7 @@ public: CameraDeviceClient(const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int cameraFacing, int clientPid, diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.cpp b/services/camera/libcameraservice/common/Camera2ClientBase.cpp index 78feb3e813..8792f9a160 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.cpp +++ b/services/camera/libcameraservice/common/Camera2ClientBase.cpp @@ -44,13 +44,14 @@ Camera2ClientBase::Camera2ClientBase( const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int api1CameraId, int cameraFacing, int clientPid, uid_t clientUid, int servicePid): - TClientBase(cameraService, remoteCallback, clientPackageName, + TClientBase(cameraService, remoteCallback, clientPackageName, clientFeatureId, cameraId, api1CameraId, cameraFacing, clientPid, clientUid, servicePid), mSharedCameraCallbacks(remoteCallback), mDeviceVersion(cameraService->getDeviceVersion(TClientBase::mCameraIdStr)), diff --git a/services/camera/libcameraservice/common/Camera2ClientBase.h b/services/camera/libcameraservice/common/Camera2ClientBase.h index 6693847aec..12cba0b1dc 100644 --- a/services/camera/libcameraservice/common/Camera2ClientBase.h +++ b/services/camera/libcameraservice/common/Camera2ClientBase.h @@ -48,6 +48,7 @@ public: Camera2ClientBase(const sp& cameraService, const sp& remoteCallback, const String16& clientPackageName, + const std::unique_ptr& clientFeatureId, const String8& cameraId, int api1CameraId, int cameraFacing, diff --git a/services/camera/libcameraservice/hidl/HidlCameraService.cpp b/services/camera/libcameraservice/hidl/HidlCameraService.cpp index 1daa035aec..97ba9c4176 100644 --- a/services/camera/libcameraservice/hidl/HidlCameraService.cpp +++ b/services/camera/libcameraservice/hidl/HidlCameraService.cpp @@ -103,7 +103,7 @@ Return HidlCameraService::connectDevice(const sp& h } sp callbacks = hybridCallbacks; binder::Status serviceRet = mAidlICameraService->connectDevice( - callbacks, String16(cameraId.c_str()), String16(""), + callbacks, String16(cameraId.c_str()), String16(""), std::unique_ptr(), hardware::ICameraService::USE_CALLING_UID, /*out*/&deviceRemote); HStatus status = HStatus::NO_ERROR; if (!serviceRet.isOk()) {