Merge "Kill app when OP_MANAGE_EXTERNAL_STORAGE is denied."

1 files changed, 30 insertions, 13 deletions

diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java
index a04f25b0d20c..e17dde9a766e 100644
--- a/services/core/java/com/android/server/StorageManagerService.java
+++ b/services/core/java/com/android/server/StorageManagerService.java
@@ -24,6 +24,7 @@ import static android.Manifest.permission.WRITE_MEDIA_STORAGE;
 import static android.app.ActivityManager.PROCESS_STATE_NONEXISTENT;
 import static android.app.AppOpsManager.MODE_ALLOWED;
 import static android.app.AppOpsManager.OP_LEGACY_STORAGE;
+import static android.app.AppOpsManager.OP_MANAGE_EXTERNAL_STORAGE;
 import static android.app.AppOpsManager.OP_READ_EXTERNAL_STORAGE;
 import static android.app.AppOpsManager.OP_REQUEST_INSTALL_PACKAGES;
 import static android.app.AppOpsManager.OP_WRITE_EXTERNAL_STORAGE;
@@ -4320,21 +4321,37 @@ class StorageManagerService extends IStorageManager.Stub
             return true;
         }
 
-        public void onAppOpsChanged(int code, int uid,
-                @Nullable String packageName, int mode) {
-            if (code == OP_REQUEST_INSTALL_PACKAGES && mIsFuseEnabled) {
-                // When using FUSE, we basically have no other choice but to kill the app
-                // after the app op is either granted or rejected.
-                final IActivityManager am = ActivityManager.getService();
-                try {
-                    am.killApplication(packageName,
-                            UserHandle.getAppId(uid),
-                            UserHandle.USER_ALL, AppOpsManager.opToName(code) + " changed.");
-                } catch (RemoteException e) {
-                }
+        private void killAppForOpChange(int code, int uid, String packageName) {
+            final IActivityManager am = ActivityManager.getService();
+            try {
+                am.killApplication(packageName,
+                        UserHandle.getAppId(uid),
+                        UserHandle.USER_ALL, AppOpsManager.opToName(code) + " changed.");
+            } catch (RemoteException e) {
+            }
+        }
 
-                return;
+        public void onAppOpsChanged(int code, int uid, @Nullable String packageName, int mode) {
+            if (mIsFuseEnabled) {
+                // When using FUSE, we may need to kill the app if the op changes
+                switch(code) {
+                    case OP_REQUEST_INSTALL_PACKAGES:
+                        // Always kill regardless of op change, to remount apps /storage
+                        killAppForOpChange(code, uid, packageName);
+                        return;
+                    case OP_MANAGE_EXTERNAL_STORAGE:
+                        if (mode != MODE_ALLOWED) {
+                            // Only kill if op is denied, to lose external_storage gid
+                            // Killing when op is granted to pickup the gid automatically, results
+                            // in a bad UX, especially since the gid only gives access to unreliable
+                            // volumes, USB OTGs that are rarely mounted. The app will get the
+                            // external_storage gid on next organic restart.
+                            killAppForOpChange(code, uid, packageName);
+                            return;
+                        }
+                }
             }
+
             if (mode == MODE_ALLOWED && (code == OP_READ_EXTERNAL_STORAGE
                     || code == OP_WRITE_EXTERNAL_STORAGE
                     || code == OP_REQUEST_INSTALL_PACKAGES)) {