new Permission class used to improve permission checks speed (by caching results)

8 files changed, 197 insertions, 42 deletions

diff --git a/include/binder/IServiceManager.h b/include/binder/IServiceManager.h
index ea149dd..24e9e99 100644
--- a/include/binder/IServiceManager.h
+++ b/include/binder/IServiceManager.h
@@ -78,6 +78,8 @@ status_t getService(const String16& name, sp<INTERFACE>* outService)
 bool checkCallingPermission(const String16& permission);
 bool checkCallingPermission(const String16& permission,
                             int32_t* outPid, int32_t* outUid);
+bool checkPermission(const String16& permission, pid_t pid, uid_t uid);
+
 
 // ----------------------------------------------------------------------
 
diff --git a/include/binder/Permission.h b/include/binder/Permission.h
new file mode 100644
index 0000000..9542d50
--- /dev/null
+++ b/include/binder/Permission.h
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef BINDER_PERMISSION_H
+#define BINDER_PERMISSION_H
+
+#include <stdint.h>
+#include <unistd.h>
+
+#include <utils/SortedVector.h>
+#include <utils/String16.h>
+#include <utils/threads.h>
+
+namespace android {
+// ---------------------------------------------------------------------------
+
+/*
+ * Permission caches the result of the permission check for the given
+ * permission name and the provided uid/pid. It also handles a few
+ * known cases efficiently (caller is in the same process or is root).
+ * The package manager does something similar but lives in dalvik world
+ * and is therefore extremely slow to access.
+ */
+
+class Permission
+{
+public:
+            Permission(char const* name);
+            Permission(const String16& name);
+            Permission(const Permission& rhs);
+    virtual ~Permission();
+
+    bool operator < (const Permission& rhs) const;
+
+    // checks the current binder call's caller has access to this permission
+    bool checkCalling() const;
+    
+    // checks the specified pid/uid has access to this permission
+    bool check(pid_t pid, uid_t uid) const;
+    
+protected:
+    virtual bool doCheckPermission(pid_t pid, uid_t uid) const;
+
+private:
+    Permission& operator = (const Permission& rhs) const;
+    const String16 mPermissionName;
+    mutable SortedVector<uid_t> mGranted;
+    const pid_t mPid;
+    mutable Mutex mLock;
+};
+
+// ---------------------------------------------------------------------------
+}; // namespace android
+
+#endif /* BINDER_PERMISSION_H */
diff --git a/libs/binder/Android.mk b/libs/binder/Android.mk
index c4d695e..2df6775 100644
--- a/libs/binder/Android.mk
+++ b/libs/binder/Android.mk
@@ -29,6 +29,7 @@ LOCAL_SRC_FILES:= \
     MemoryHeapBase.cpp \
     MemoryHeapPmem.cpp \
 	Parcel.cpp \
+	Permission.cpp \
 	ProcessState.cpp \
 	Static.cpp
 
diff --git a/libs/binder/IServiceManager.cpp b/libs/binder/IServiceManager.cpp
index 88774e7..0cf4158 100644
--- a/libs/binder/IServiceManager.cpp
+++ b/libs/binder/IServiceManager.cpp
@@ -19,8 +19,8 @@
 #include <binder/IServiceManager.h>
 
 #include <utils/Debug.h>
-#include <binder/IPCThreadState.h>
 #include <utils/Log.h>
+#include <binder/IPCThreadState.h>
 #include <binder/Parcel.h>
 #include <utils/String8.h>
 #include <utils/SystemClock.h>
@@ -53,14 +53,19 @@ bool checkCallingPermission(const String16& permission)
 
 static String16 _permission("permission");
 
+
 bool checkCallingPermission(const String16& permission, int32_t* outPid, int32_t* outUid)
 {
     IPCThreadState* ipcState = IPCThreadState::self();
-    int32_t pid = ipcState->getCallingPid();
-    int32_t uid = ipcState->getCallingUid();
+    pid_t pid = ipcState->getCallingPid();
+    uid_t uid = ipcState->getCallingUid();
     if (outPid) *outPid = pid;
-    if (outUid) *outUid= uid;
-    
+    if (outUid) *outUid = uid;
+    return checkPermission(permission, pid, uid);
+}
+
+bool checkPermission(const String16& permission, pid_t pid, uid_t uid)
+{
     sp<IPermissionController> pc;
     gDefaultServiceManagerLock.lock();
     pc = gPermissionController;
diff --git a/libs/binder/Permission.cpp b/libs/binder/Permission.cpp
new file mode 100644
index 0000000..fd8fe69
--- /dev/null
+++ b/libs/binder/Permission.cpp
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <stdint.h>
+#include <utils/Log.h>
+#include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
+#include <binder/Permission.h>
+
+namespace android {
+// ---------------------------------------------------------------------------
+
+Permission::Permission(char const* name)
+    : mPermissionName(name), mPid(getpid())
+{
+}
+
+Permission::Permission(const String16& name)
+    : mPermissionName(name), mPid(getpid())
+{
+}
+
+Permission::Permission(const Permission& rhs)
+    : mPermissionName(rhs.mPermissionName),
+    mGranted(rhs.mGranted),
+    mPid(rhs.mPid)
+{
+}
+
+Permission::~Permission()
+{
+}
+
+bool Permission::operator < (const Permission& rhs) const
+{
+    return mPermissionName < rhs.mPermissionName;
+}
+
+bool Permission::checkCalling() const
+{
+    IPCThreadState* ipcState = IPCThreadState::self();
+    pid_t pid = ipcState->getCallingPid();
+    uid_t uid = ipcState->getCallingUid();
+    return doCheckPermission(pid, uid);
+}
+
+bool Permission::check(pid_t pid, uid_t uid) const
+{
+    return doCheckPermission(pid, uid);
+}
+
+bool Permission::doCheckPermission(pid_t pid, uid_t uid) const
+{
+    if ((uid == 0) || (pid == mPid)) {
+        // root and ourselves is always okay
+        return true;
+    } else {
+        // see if we already granted this permission for this uid
+        Mutex::Autolock _l(mLock);
+        if (mGranted.indexOf(uid) >= 0)
+            return true;
+    }
+
+    bool granted = checkPermission(mPermissionName, pid, uid);
+    if (granted) {
+        Mutex::Autolock _l(mLock);
+        // no need to check again, the old item will be replaced if it is
+        // already there.
+        mGranted.add(uid);
+    }
+    return granted;
+}
+
+// ---------------------------------------------------------------------------
+}; // namespace android
diff --git a/libs/surfaceflinger/LayerBase.cpp b/libs/surfaceflinger/LayerBase.cpp
index c678711..b65c983 100644
--- a/libs/surfaceflinger/LayerBase.cpp
+++ b/libs/surfaceflinger/LayerBase.cpp
@@ -729,27 +729,20 @@ void LayerBaseClient::Surface::getSurfaceData(
 }
 
 status_t LayerBaseClient::Surface::onTransact(
-    uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
+        uint32_t code, const Parcel& data, Parcel* reply, uint32_t flags)
 {
     switch (code) {
         case REGISTER_BUFFERS:
         case UNREGISTER_BUFFERS:
         case CREATE_OVERLAY:
         {
-            // codes that require permission check
-            IPCThreadState* ipc = IPCThreadState::self();
-            const int pid = ipc->getCallingPid();
-            const int self_pid = getpid();
-            if (LIKELY(pid != self_pid)) {
-                // we're called from a different process, do the real check
-                if (!checkCallingPermission(
-                        String16("android.permission.ACCESS_SURFACE_FLINGER")))
-                {
-                    const int uid = ipc->getCallingUid();
-                    LOGE("Permission Denial: "
-                            "can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
-                    return PERMISSION_DENIED;
-                }
+            if (!mFlinger->mAccessSurfaceFlinger.checkCalling()) {
+                IPCThreadState* ipc = IPCThreadState::self();
+                const int pid = ipc->getCallingPid();
+                const int uid = ipc->getCallingUid();
+                LOGE("Permission Denial: "
+                        "can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
+                return PERMISSION_DENIED;
             }
         }
     }
diff --git a/libs/surfaceflinger/SurfaceFlinger.cpp b/libs/surfaceflinger/SurfaceFlinger.cpp
index 0d1be2d..a56ecd8 100644
--- a/libs/surfaceflinger/SurfaceFlinger.cpp
+++ b/libs/surfaceflinger/SurfaceFlinger.cpp
@@ -175,6 +175,9 @@ SurfaceFlinger::SurfaceFlinger()
         mTransactionCount(0),
         mLayersRemoved(false),
         mBootTime(systemTime()),
+        mHardwareTest("android.permission.HARDWARE_TEST"),
+        mAccessSurfaceFlinger("android.permission.ACCESS_SURFACE_FLINGER"),
+        mDump("android.permission.DUMP"),
         mLastScheduledBroadcast(NULL),
         mVisibleRegionsDirty(false),
         mDeferReleaseConsole(false),
@@ -1430,9 +1433,7 @@ status_t SurfaceFlinger::dump(int fd, const Vector<String16>& args)
     const size_t SIZE = 1024;
     char buffer[SIZE];
     String8 result;
-    if (checkCallingPermission(
-            String16("android.permission.DUMP")) == false)
-    { // not allowed
+    if (!mDump.checkCalling()) {
         snprintf(buffer, SIZE, "Permission Denial: "
                 "can't dump SurfaceFlinger from pid=%d, uid=%d\n",
                 IPCThreadState::self()->getCallingPid(),
@@ -1534,29 +1535,21 @@ status_t SurfaceFlinger::onTransact(
             IPCThreadState* ipc = IPCThreadState::self();
             const int pid = ipc->getCallingPid();
             const int uid = ipc->getCallingUid();
-            const int self_pid = getpid();
-            if (UNLIKELY(pid != self_pid && uid != AID_GRAPHICS)) {
-                // we're called from a different process, do the real check
-                if (!checkCallingPermission(
-                        String16("android.permission.ACCESS_SURFACE_FLINGER")))
-                {
-                    LOGE("Permission Denial: "
-                            "can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
-                    return PERMISSION_DENIED;
-                }
+            if ((uid != AID_GRAPHICS) && !mAccessSurfaceFlinger.check(pid, uid)) {
+                LOGE("Permission Denial: "
+                        "can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
+                return PERMISSION_DENIED;
             }
         }
     }
-
     status_t err = BnSurfaceComposer::onTransact(code, data, reply, flags);
     if (err == UNKNOWN_TRANSACTION || err == PERMISSION_DENIED) {
-        // HARDWARE_TEST stuff...
-        if (UNLIKELY(checkCallingPermission(
-                String16("android.permission.HARDWARE_TEST")) == false))
-        { // not allowed
-            LOGE("Permission Denial: pid=%d, uid=%d\n",
-                    IPCThreadState::self()->getCallingPid(),
-                    IPCThreadState::self()->getCallingUid());
+        if (UNLIKELY(!mHardwareTest.checkCalling())) {
+            IPCThreadState* ipc = IPCThreadState::self();
+            const int pid = ipc->getCallingPid();
+            const int uid = ipc->getCallingUid();
+            LOGE("Permission Denial: "
+                    "can't access SurfaceFlinger pid=%d, uid=%d", pid, uid);
             return PERMISSION_DENIED;
         }
         int n;
diff --git a/libs/surfaceflinger/SurfaceFlinger.h b/libs/surfaceflinger/SurfaceFlinger.h
index a21e186..ebe315a 100644
--- a/libs/surfaceflinger/SurfaceFlinger.h
+++ b/libs/surfaceflinger/SurfaceFlinger.h
@@ -25,9 +25,11 @@
 #include <utils/threads.h>
 #include <utils/Atomic.h>
 #include <utils/Errors.h>
-#include <binder/MemoryDealer.h>
 #include <utils/RefBase.h>
 
+#include <binder/MemoryDealer.h>
+#include <binder/Permission.h>
+
 #include <ui/PixelFormat.h>
 #include <ui/ISurfaceComposer.h>
 #include <ui/ISurfaceFlingerClient.h>
@@ -321,6 +323,9 @@ private:
                 surface_flinger_cblk_t*     mServerCblk;
                 GLuint                      mWormholeTexName;
                 nsecs_t                     mBootTime;
+                Permission                  mHardwareTest;
+                Permission                  mAccessSurfaceFlinger;
+                Permission                  mDump;
                 
                 // Can only accessed from the main thread, these members
                 // don't need synchronization