commit f1121dc1d35c7e8c317c278aad0dd4ad1358d870
author Kenny Root <kroot@google.com> Wed Sep 29 07:30:53 2010 -0700
committer Kenny Root <kroot@google.com> Wed Sep 29 07:38:55 2010 -0700
tree 328181f28c37eb9113ad04035f9dfa70a1df2ad0
parent 05105f7abe02b2dff91d6260b3628c8b97816bab

Move all the permissions check up for unmount

We have already scanned the state of the OBB in mountObb, so check the
caller against the stored state in the unmountObb call. This allows us
to ensure the calling binder is the same one that mounted it since we
tie the lifecycle of the OBB to the lifecycle of the binder.

Change-Id: I45d9cfbab5d3f5b37a6a9b594b10bd8b91cccc45

services/java/com/android/server/MountService.java

diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index f3625a8..265d6138 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -1451,11 +1451,6 @@
         mHandler.sendEmptyMessage(H_UNMOUNT_PM_DONE);
     }
 
-    private boolean isCallerOwnerOfPackageOrSystem(String packageName) {
-        final int callerUid = Binder.getCallingUid();
-        return isUidOwnerOfPackageOrSystem(packageName, callerUid);
-    }
-
     private boolean isUidOwnerOfPackageOrSystem(String packageName, int callerUid) {
         if (callerUid == android.os.Process.SYSTEM_UID) {
             return true;
@@ -1507,6 +1502,12 @@
         waitForReady();
         warnOnNotMounted();
 
+        if (filename == null) {
+            throw new IllegalArgumentException("filename cannot be null");
+        } else if (token == null) {
+            throw new IllegalArgumentException("token cannot be null");
+        }
+
         final ObbState obbState;
 
         synchronized (mObbMounts) {
@@ -1533,6 +1534,12 @@
     }
 
     public void unmountObb(String filename, boolean force, IObbActionListener token) {
+        if (filename == null) {
+            throw new IllegalArgumentException("filename cannot be null");
+        } else if (token == null) {
+            throw new IllegalArgumentException("token cannot be null");
+        }
+
         final ObbState obbState;
 
         synchronized (mObbMounts) {
@@ -1540,6 +1547,12 @@
                 throw new IllegalArgumentException("OBB is not mounted");
             }
             obbState = mObbPathToStateMap.get(filename);
+
+            if (Binder.getCallingUid() != obbState.callerUid) {
+                throw new SecurityException("caller UID does not match original mount caller UID");
+            } else if (!token.asBinder().equals(obbState.token.asBinder())) {
+                throw new SecurityException("caller does not match original mount caller");
+            }
         }
 
         UnmountObbAction action = new UnmountObbAction(obbState, force);
@@ -1758,9 +1771,9 @@
         }
 
         public void handleExecute() throws RemoteException, IOException {
-            ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
+            final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
             if (obbInfo == null) {
-                throw new IOException("Couldn't read OBB file");
+                throw new IOException("Couldn't read OBB file: " + mObbState.filename);
             }
 
             if (!isUidOwnerOfPackageOrSystem(obbInfo.packageName, mObbState.callerUid)) {
@@ -1833,13 +1846,9 @@
         }
 
         public void handleExecute() throws RemoteException, IOException {
-            ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
+            final ObbInfo obbInfo = mContainerService.getObbInfo(mObbState.filename);
             if (obbInfo == null) {
-                throw new IOException("Couldn't read OBB file");
-            }
-
-            if (!isCallerOwnerOfPackageOrSystem(obbInfo.packageName)) {
-                throw new IllegalArgumentException("Caller package does not match OBB file");
+                throw new IOException("Couldn't read OBB file: " + mObbState.filename);
             }
 
             int rc = StorageResultCode.OperationSucceeded;