Camera: Limit valid caller PIDs for camera clients.

Narrow down on PID checks to avoid cases where service can access the
camera even though it shouldn't be able to, per API semantics.

Bug: 6970469

Change-Id: Ic468a31949c28ef978b6ed48a70e4601c7ced684

diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp
index 7a6e344..cd9da42 100644 (file)
--- a/services/camera/libcameraservice/Camera2Client.cpp
+++ b/services/camera/libcameraservice/Camera2Client.cpp
@@ -65,10 +65,10 @@ Camera2Client::Camera2Client(const sp<CameraService>& cameraService,
 
 status_t Camera2Client::checkPid(const char* checkLocation) const {
     int callingPid = getCallingPid();
-    if (callingPid == mClientPid || callingPid == mServicePid) return NO_ERROR;
+    if (callingPid == mClientPid) return NO_ERROR;
 
     ALOGE("%s: attempt to use a locked camera from a different process"
-            " (old pid %d, new pid %d, servicePid %d)", checkLocation, mClientPid, callingPid, mServicePid);
+            " (old pid %d, new pid %d)", checkLocation, mClientPid, callingPid);
     return PERMISSION_DENIED;
 }
 
@@ -139,19 +139,7 @@ Camera2Client::~Camera2Client() {
 
     mDestructionStarted = true;
 
-    Parameters::State state;
-    // warning:
-    //   holding on to locks more than necessary may be hazardous to your health
-    {
-        SharedParameters::Lock l(mParameters);
-        state = l.mParameters.state;
-    }
-
-    if (state != Parameters::DISCONNECTED) {
-        // Rewrite mClientPid to allow shutdown by CameraService
-        mClientPid = getCallingPid();
-        disconnect();
-    }
+    disconnect();
 
     ALOGI("Camera %d: Closed", mCameraId);
 }
@@ -372,7 +360,10 @@ void Camera2Client::disconnect() {
     ATRACE_CALL();
     Mutex::Autolock icl(mICameraLock);
     status_t res;
-    if ( (res = checkPid(__FUNCTION__) ) != OK) return;
+
+    // Allow both client and the media server to disconnect at all times
+    int callingPid = getCallingPid();
+    if (callingPid != mClientPid && callingPid != mServicePid) return;
 
     if (mDevice == 0) return;
 
@@ -382,6 +373,7 @@ void Camera2Client::disconnect() {
 
     {
         SharedParameters::Lock l(mParameters);
+        if (l.mParameters.state == Parameters::DISCONNECTED) return;
         l.mParameters.state = Parameters::DISCONNECTED;
     }
 

diff --git a/services/camera/libcameraservice/CameraClient.cpp b/services/camera/libcameraservice/CameraClient.cpp
index 7e199fa..b930c02 100644 (file)
--- a/services/camera/libcameraservice/CameraClient.cpp
+++ b/services/camera/libcameraservice/CameraClient.cpp
@@ -102,8 +102,6 @@ CameraClient::~CameraClient() {
     int callingPid = getCallingPid();
     LOG1("CameraClient::~CameraClient E (pid %d, this %p)", callingPid, this);
 
-    // set mClientPid to let disconnet() tear down the hardware
-    mClientPid = callingPid;
     disconnect();
     LOG1("CameraClient::~CameraClient X (pid %d, this %p)", callingPid, this);
 }
@@ -125,7 +123,7 @@ status_t CameraClient::dump(int fd, const Vector<String16>& args) {
 
 status_t CameraClient::checkPid() const {
     int callingPid = getCallingPid();
-    if (callingPid == mClientPid || callingPid == mServicePid) return NO_ERROR;
+    if (callingPid == mClientPid) return NO_ERROR;
 
     ALOGW("attempt to use a locked camera from a different process"
          " (old pid %d, new pid %d)", mClientPid, callingPid);
@@ -219,7 +217,8 @@ void CameraClient::disconnect() {
     LOG1("disconnect E (pid %d)", callingPid);
     Mutex::Autolock lock(mLock);
 
-    if (checkPid() != NO_ERROR) {
+    // Allow both client and the media server to disconnect at all times
+    if (callingPid != mClientPid && callingPid != mServicePid) {
         ALOGW("different client - don't disconnect");
         return;
     }