From 782dc19e95a2bfde833a0803c1d8ab77d047c11c Mon Sep 17 00:00:00 2001 From: Zim Date: Wed, 17 Mar 2021 11:59:22 +0000 Subject: [PATCH] Fix MANAGE_EXTERNAL_STORAGE permission gid mapping In Android R, we introduced a platform.xml based mapping. Unfortunately, it only worked for signature|preinstalled apps. To support apps granted the appop (via special app access permissions), we now check the permission and appop grant state explicitly and grant the app the external_storage gid appropriately Test: Manual Bug: 165515144 Change-Id: Ib91e1b3a7e54ac2c83fb1d94446bed06fd44bcf6 --- core/java/android/os/Process.java | 6 ++++++ .../os/storage/StorageManagerInternal.java | 11 ++++++++++ data/etc/platform.xml | 4 ---- .../android/server/StorageManagerService.java | 20 +++++++++++++++++++ .../com/android/server/am/ProcessList.java | 13 ++++++++++-- 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/core/java/android/os/Process.java b/core/java/android/os/Process.java index e75e224d9a6f5..65cfef47da2af 100644 --- a/core/java/android/os/Process.java +++ b/core/java/android/os/Process.java @@ -224,6 +224,12 @@ public class Process { */ public static final int FSVERITY_CERT_UID = 1075; + /** + * GID that gives access to USB OTG (unreliable) volumes on /mnt/media_rw/ + * @hide + */ + public static final int EXTERNAL_STORAGE_GID = 1077; + /** * GID that gives write access to app-private data directories on external * storage (used on devices without sdcardfs only). diff --git a/core/java/android/os/storage/StorageManagerInternal.java b/core/java/android/os/storage/StorageManagerInternal.java index 82c4c715f4b0f..54905ec6eaeb6 100644 --- a/core/java/android/os/storage/StorageManagerInternal.java +++ b/core/java/android/os/storage/StorageManagerInternal.java @@ -38,6 +38,17 @@ public abstract class StorageManagerInternal { */ public abstract int getExternalStorageMountMode(int uid, String packageName); + /** + * Checks whether the {@code packageName} with {@code uid} has full external storage access via + * the {@link MANAGE_EXTERNAL_STORAGE} permission. + * + * @param uid the UID for which to check access. + * @param packageName the package in the UID for making the call. + * @return whether the {@code packageName} has full external storage access. + * Returns {@code true} if it has access, {@code false} otherwise. + */ + public abstract boolean hasExternalStorageAccess(int uid, String packageName); + /** * A listener for reset events in the StorageManagerService. */ diff --git a/data/etc/platform.xml b/data/etc/platform.xml index b3a180d70fe26..d87f4a69df7b9 100644 --- a/data/etc/platform.xml +++ b/data/etc/platform.xml @@ -60,10 +60,6 @@ - - - - diff --git a/services/core/java/com/android/server/StorageManagerService.java b/services/core/java/com/android/server/StorageManagerService.java index c732e8a26b379..a19090396467e 100644 --- a/services/core/java/com/android/server/StorageManagerService.java +++ b/services/core/java/com/android/server/StorageManagerService.java @@ -18,6 +18,7 @@ package com.android.server; import static android.Manifest.permission.ACCESS_MTP; import static android.Manifest.permission.INSTALL_PACKAGES; +import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE; import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE; import static android.app.AppOpsManager.MODE_ALLOWED; import static android.app.AppOpsManager.OP_LEGACY_STORAGE; @@ -4605,6 +4606,25 @@ class StorageManagerService extends IStorageManager.Stub return mode; } + @Override + public boolean hasExternalStorageAccess(int uid, String packageName) { + try { + if (mIPackageManager.checkUidPermission( + MANAGE_EXTERNAL_STORAGE, uid) == PERMISSION_GRANTED) { + return true; + } + + if (mIAppOpsService.checkOperation( + OP_MANAGE_EXTERNAL_STORAGE, uid, packageName) == MODE_ALLOWED) { + return true; + } + } catch (RemoteException e) { + Slog.w("Failed to check MANAGE_EXTERNAL_STORAGE access for " + packageName, e); + } + + return false; + } + @Override public void addResetListener(StorageManagerInternal.ResetListener listener) { synchronized (mResetListeners) { diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java index 4972aa7104970..442cdd9222132 100644 --- a/services/core/java/com/android/server/am/ProcessList.java +++ b/services/core/java/com/android/server/am/ProcessList.java @@ -1598,7 +1598,8 @@ public final class ProcessList { } } - private int[] computeGidsForProcess(int mountExternal, int uid, int[] permGids) { + private int[] computeGidsForProcess(int mountExternal, int uid, int[] permGids, + boolean externalStorageAccess) { ArrayList gidList = new ArrayList<>(permGids.length + 5); final int sharedAppGid = UserHandle.getSharedAppGid(UserHandle.getAppId(uid)); @@ -1644,6 +1645,11 @@ public final class ProcessList { // PublicVolumes: /mnt/media_rw/ gidList.add(Process.MEDIA_RW_GID); } + if (externalStorageAccess) { + // Apps with MANAGE_EXTERNAL_STORAGE PERMISSION need the external_storage gid to access + // USB OTG (unreliable) volumes on /mnt/media_rw/ + gidList.add(Process.EXTERNAL_STORAGE_GID); + } int[] gidArray = new int[gidList.size()]; for (int i = 0; i < gidArray.length; i++) { @@ -1805,6 +1811,7 @@ public final class ProcessList { int uid = app.uid; int[] gids = null; int mountExternal = Zygote.MOUNT_EXTERNAL_NONE; + boolean externalStorageAccess = false; if (!app.isolated) { int[] permGids = null; try { @@ -1816,6 +1823,8 @@ public final class ProcessList { StorageManagerInternal.class); mountExternal = storageManagerInternal.getExternalStorageMountMode(uid, app.info.packageName); + externalStorageAccess = storageManagerInternal.hasExternalStorageAccess(uid, + app.info.packageName); } catch (RemoteException e) { throw e.rethrowAsRuntimeException(); } @@ -1835,7 +1844,7 @@ public final class ProcessList { } } - gids = computeGidsForProcess(mountExternal, uid, permGids); + gids = computeGidsForProcess(mountExternal, uid, permGids, externalStorageAccess); } app.setMountMode(mountExternal); checkSlow(startTime, "startProcess: building args");