summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Sudheer Shanka <sudheersai@google.com> 2019-05-16 16:55:50 -0700
committer Sudheer Shanka <sudheersai@google.com> 2019-05-20 10:57:44 -0700
commitb1613982d04f78f0cd6ab5929cf33ce77b6e4ca9 (patch)
treeb49adbfd2736f5bd746e12f281e87c02cdf47b70
parent24d193cc301621ce9866a39effcb74621e2fef67 (diff)
Notify StorageManagerService when storage related app ops change.
StorageManagerService needs to trigger update of storage mountpoints when an app gets storage access, so update AppOpsService to notify StorageManagerService synchronously when storage related app ops change. Also, when an app gets REQUEST_INSTALL_PACKAGES appop denied, kill the app. Bug: 132466536 Test: manual Test: atest cts/hostsidetests/appsecurity/src/android/appsecurity/cts/ExternalStorageHostTest.java Change-Id: I130dde1bcffea6c96e5d8c173055737850af6151
-rw-r--r--core/java/android/os/storage/StorageManagerInternal.java8
-rw-r--r--services/core/java/com/android/server/StorageManagerService.java64
-rw-r--r--services/core/java/com/android/server/appop/AppOpsService.java13
3 files changed, 82 insertions, 3 deletions
diff --git a/core/java/android/os/storage/StorageManagerInternal.java b/core/java/android/os/storage/StorageManagerInternal.java
index 942bf94b1db6..14c299d11a94 100644
--- a/core/java/android/os/storage/StorageManagerInternal.java
+++ b/core/java/android/os/storage/StorageManagerInternal.java
@@ -16,6 +16,7 @@
package android.os.storage;
+import android.annotation.Nullable;
import android.os.IVold;
/**
@@ -101,4 +102,11 @@ public abstract class StorageManagerInternal {
* @param listener The listener that will be notified on reset events.
*/
public abstract void addResetListener(ResetListener listener);
+
+ /**
+ * Notified when any app op changes so that storage mount points can be updated if the app op
+ * affects them.
+ */
+ public abstract void onAppOpsChanged(int code, int uid,
+ @Nullable String packageName, int mode);
}
diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index fc355b7cce2f..9ebbfa0b0d4c 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -96,6 +96,7 @@ import android.os.SystemClock;
import android.os.SystemProperties;
import android.os.UserHandle;
import android.os.UserManager;
+import android.os.UserManagerInternal;
import android.os.storage.DiskInfo;
import android.os.storage.IObbActionListener;
import android.os.storage.IStorageEventListener;
@@ -3242,7 +3243,22 @@ class StorageManagerService extends IStorageManager.Stub
public void opChanged(int op, int uid, String packageName) throws RemoteException {
if (!ENABLE_ISOLATED_STORAGE) return;
- remountUidExternalStorage(uid, getMountMode(uid, packageName));
+ if (op == OP_REQUEST_INSTALL_PACKAGES) {
+ // Only handling the case when the appop is denied. The other cases will be
+ // handled in the synchronous callback from AppOpsService.
+ if (packageName != null && mIAppOpsService.checkOperation(
+ OP_REQUEST_INSTALL_PACKAGES, uid, packageName) != MODE_ALLOWED) {
+ try {
+ ActivityManager.getService().killUid(
+ UserHandle.getAppId(uid), UserHandle.getUserId(uid),
+ "OP_REQUEST_INSTALL_PACKAGES is denied");
+ } catch (RemoteException e) {
+ // same process - should not happen
+ }
+ }
+ } else {
+ remountUidExternalStorage(uid, getMountMode(uid, packageName));
+ }
}
};
@@ -3579,6 +3595,12 @@ class StorageManagerService extends IStorageManager.Stub
if (Process.isIsolated(uid)) {
return Zygote.MOUNT_EXTERNAL_NONE;
}
+
+ final String[] packagesForUid = mIPackageManager.getPackagesForUid(uid);
+ if (packageName == null) {
+ packageName = packagesForUid[0];
+ }
+
if (mPmInternal.isInstantApp(packageName, UserHandle.getUserId(uid))) {
return Zygote.MOUNT_EXTERNAL_DEFAULT;
}
@@ -3601,8 +3623,18 @@ class StorageManagerService extends IStorageManager.Stub
// runtime permission; this is a firm CDD requirement
final boolean hasInstall = mIPackageManager.checkUidPermission(INSTALL_PACKAGES,
uid) == PERMISSION_GRANTED;
- final boolean hasInstallOp = mIAppOpsService.checkOperation(OP_REQUEST_INSTALL_PACKAGES,
- uid, packageName) == MODE_ALLOWED;
+ boolean hasInstallOp = false;
+ // OP_REQUEST_INSTALL_PACKAGES is granted/denied per package but vold can't
+ // update mountpoints of a specific package. So, check the appop for all packages
+ // sharing the uid and allow same level of storage access for all packages even if
+ // one of the packages has the appop granted.
+ for (String uidPackageName : packagesForUid) {
+ if (mIAppOpsService.checkOperation(
+ OP_REQUEST_INSTALL_PACKAGES, uid, uidPackageName) == MODE_ALLOWED) {
+ hasInstallOp = true;
+ break;
+ }
+ }
if ((hasInstall || hasInstallOp) && hasWrite) {
return Zygote.MOUNT_EXTERNAL_WRITE;
}
@@ -3870,6 +3902,14 @@ class StorageManagerService extends IStorageManager.Stub
if (ENABLE_ISOLATED_STORAGE) {
return getMountMode(uid, packageName);
}
+ try {
+ if (packageName == null) {
+ final String[] packagesForUid = mIPackageManager.getPackagesForUid(uid);
+ packageName = packagesForUid[0];
+ }
+ } catch (RemoteException e) {
+ // Should not happen - same process
+ }
// No locking - CopyOnWriteArrayList
int mountMode = Integer.MAX_VALUE;
for (ExternalStorageMountPolicy policy : mPolicies) {
@@ -3918,5 +3958,23 @@ class StorageManagerService extends IStorageManager.Stub
}
return true;
}
+
+ public void onAppOpsChanged(int code, int uid,
+ @Nullable String packageName, int mode) {
+ if (mode == MODE_ALLOWED && (code == OP_READ_EXTERNAL_STORAGE
+ || code == OP_WRITE_EXTERNAL_STORAGE
+ || code == OP_REQUEST_INSTALL_PACKAGES)) {
+ final long token = Binder.clearCallingIdentity();
+ try {
+ final UserManagerInternal userManagerInternal =
+ LocalServices.getService(UserManagerInternal.class);
+ if (userManagerInternal.isUserInitialized(UserHandle.getUserId(uid))) {
+ onExternalStoragePolicyChanged(uid, packageName);
+ }
+ } finally {
+ Binder.restoreCallingIdentity(token);
+ }
+ }
+ }
}
}
diff --git a/services/core/java/com/android/server/appop/AppOpsService.java b/services/core/java/com/android/server/appop/AppOpsService.java
index 448e7738203a..3bed9c3dc7e1 100644
--- a/services/core/java/com/android/server/appop/AppOpsService.java
+++ b/services/core/java/com/android/server/appop/AppOpsService.java
@@ -1314,6 +1314,7 @@ public class AppOpsService extends IAppOpsService.Stub {
}
if (callbackSpecs == null) {
+ notifyOpChangedSync(code, uid, null, mode);
return;
}
@@ -1335,6 +1336,16 @@ public class AppOpsService extends IAppOpsService.Stub {
}
}
}
+
+ notifyOpChangedSync(code, uid, null, mode);
+ }
+
+ private void notifyOpChangedSync(int code, int uid, @NonNull String packageName, int mode) {
+ final StorageManagerInternal storageManagerInternal =
+ LocalServices.getService(StorageManagerInternal.class);
+ if (storageManagerInternal != null) {
+ storageManagerInternal.onAppOpsChanged(code, uid, packageName, mode);
+ }
}
/**
@@ -1438,6 +1449,8 @@ public class AppOpsService extends IAppOpsService.Stub {
AppOpsService::notifyOpChanged,
this, repCbs, code, uid, packageName));
}
+
+ notifyOpChangedSync(code, uid, packageName, mode);
}
private void notifyOpChanged(ArraySet<ModeCallback> callbacks, int code,