From 617048834dc5b2cb87ab208bdd526262a7c4e90c Mon Sep 17 00:00:00 2001
From: Eino-Ville Talvala <etalvala@google.com>
Date: Tue, 9 Oct 2012 22:16:58 -0700
Subject: [PATCH] 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
---
 services/camera/libcameraservice/Camera2Client.cpp | 24 ++++++++--------------
 services/camera/libcameraservice/CameraClient.cpp  |  7 +++----
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/services/camera/libcameraservice/Camera2Client.cpp b/services/camera/libcameraservice/Camera2Client.cpp
index 7a6e3445ba..cd9da42d28 100644
--- 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 7e199fa6f8..b930c02fdf 100644
--- 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;
     }
-- 
2.11.0