From 151b21b227eb9da41548f137a3a94d5c7d44eb69 Mon Sep 17 00:00:00 2001 From: Suprabh Shukla Date: Fri, 27 Apr 2018 19:30:30 -0700 Subject: [PATCH] Unsuspending packages on suspending app data clear A suspending app loses all app data, jobs, alarms, etc. when its data is cleared. The packages it suspended should get unsuspended so the device is not left in an inconsistent state. Also simplifying the permission check to block any cross user calls not from system or root. Test: Manual test by suspending with com.android.frameworks.servicestests and then clearing data via Settings and also "adb shell pm clear" Bug: 77801553 Change-Id: I20424322b5917f3b87e2cf48a6444de9f242d311 --- .../server/pm/PackageManagerService.java | 48 ++++++++++++------- .../java/com/android/server/pm/Settings.java | 16 +++---- .../permission/PermissionManagerService.java | 2 +- 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index af5521d036af2..84e9d02d6577c 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -88,11 +88,11 @@ import static android.os.storage.StorageManager.FLAG_STORAGE_CE; import static android.os.storage.StorageManager.FLAG_STORAGE_DE; import static android.system.OsConstants.O_CREAT; import static android.system.OsConstants.O_RDWR; + import static com.android.internal.app.IntentForwarderActivity.FORWARD_INTENT_TO_MANAGED_PROFILE; import static com.android.internal.app.IntentForwarderActivity.FORWARD_INTENT_TO_PARENT; import static com.android.internal.content.NativeLibraryHelper.LIB64_DIR_NAME; import static com.android.internal.content.NativeLibraryHelper.LIB_DIR_NAME; -import static com.android.internal.util.ArrayUtils.appendElement; import static com.android.internal.util.ArrayUtils.appendInt; import static com.android.server.pm.InstructionSets.getAppDexInstructionSets; import static com.android.server.pm.InstructionSets.getDexCodeInstructionSet; @@ -275,7 +275,6 @@ import com.android.internal.R; import com.android.internal.annotations.GuardedBy; import com.android.internal.app.IMediaContainerService; import com.android.internal.app.ResolverActivity; -import com.android.internal.app.SuspendedAppActivity; import com.android.internal.content.NativeLibraryHelper; import com.android.internal.content.PackageHelper; import com.android.internal.logging.MetricsLogger; @@ -411,7 +410,7 @@ public class PackageManagerService extends IPackageManager.Stub static final boolean DEBUG_DOMAIN_VERIFICATION = false; private static final boolean DEBUG_BACKUP = false; public static final boolean DEBUG_INSTALL = false; - public static final boolean DEBUG_REMOVE = true; + public static final boolean DEBUG_REMOVE = false; private static final boolean DEBUG_BROADCASTS = false; private static final boolean DEBUG_SHOW_INFO = false; private static final boolean DEBUG_PACKAGE_INFO = false; @@ -14031,13 +14030,10 @@ public class PackageManagerService extends IPackageManager.Stub + Manifest.permission.MANAGE_USERS); } final int callingUid = Binder.getCallingUid(); - mPermissionManager.enforceCrossUserPermission(callingUid, userId, - true /* requireFullPermission */, true /* checkShell */, - "setPackagesSuspended for user " + userId); - if (callingUid != Process.ROOT_UID && - !UserHandle.isSameApp(getPackageUid(callingPackage, 0, userId), callingUid)) { - throw new IllegalArgumentException("CallingPackage " + callingPackage + " does not" - + " belong to calling app id " + UserHandle.getAppId(callingUid)); + if (callingUid != Process.ROOT_UID && callingUid != Process.SYSTEM_UID + && getPackageUid(callingPackage, 0, userId) != callingUid) { + throw new SecurityException("Calling package " + callingPackage + " in user " + + userId + " does not belong to calling uid " + callingUid); } if (!PLATFORM_PACKAGE_NAME.equals(callingPackage) && mProtectedPackages.getDeviceOwnerOrProfileOwnerPackage(userId) != null) { @@ -14164,9 +14160,19 @@ public class PackageManagerService extends IPackageManager.Stub } } - void onSuspendingPackageRemoved(String packageName, int removedForUser) { - final int[] userIds = (removedForUser == UserHandle.USER_ALL) ? sUserManager.getUserIds() - : new int[] {removedForUser}; + /** + * Immediately unsuspends any packages suspended by the given package. To be called + * when such a package's data is cleared or it is removed from the device. + * + *

Should not be used on a frequent code path as it flushes state to disk + * synchronously + * + * @param packageName The package holding {@link Manifest.permission#SUSPEND_APPS} permission + * @param affectedUser The user for which the changes are taking place. + */ + void unsuspendForSuspendingPackage(String packageName, int affectedUser) { + final int[] userIds = (affectedUser == UserHandle.USER_ALL) ? sUserManager.getUserIds() + : new int[] {affectedUser}; for (int userId : userIds) { List affectedPackages = new ArrayList<>(); synchronized (mPackages) { @@ -14183,6 +14189,8 @@ public class PackageManagerService extends IPackageManager.Stub new String[affectedPackages.size()]); sendMyPackageSuspendedOrUnsuspended(packageArray, false, null, userId); sendPackagesSuspendedForUser(packageArray, userId, false, null); + // Write package restrictions immediately to avoid an inconsistent state. + mSettings.writePackageRestrictionsLPr(userId); } } } @@ -18758,7 +18766,7 @@ public class PackageManagerService extends IPackageManager.Stub final int userId = user == null ? UserHandle.USER_ALL : user.getIdentifier(); if (ps.getPermissionsState().hasPermission(Manifest.permission.SUSPEND_APPS, userId)) { - onSuspendingPackageRemoved(packageName, userId); + unsuspendForSuspendingPackage(packageName, userId); } @@ -18899,10 +18907,10 @@ public class PackageManagerService extends IPackageManager.Stub true /*notLaunched*/, false /*hidden*/, false /*suspended*/, - null, /*suspendingPackage*/ - null, /*dialogMessage*/ - null, /*suspendedAppExtras*/ - null, /*suspendedLauncherExtras*/ + null /*suspendingPackage*/, + null /*dialogMessage*/, + null /*suspendedAppExtras*/, + null /*suspendedLauncherExtras*/, false /*instantApp*/, false /*virtualPreload*/, null /*lastDisableAppCaller*/, @@ -19089,6 +19097,10 @@ public class PackageManagerService extends IPackageManager.Stub if (dsm != null) { dsm.checkMemory(); } + if (checkPermission(Manifest.permission.SUSPEND_APPS, packageName, userId) + == PERMISSION_GRANTED) { + unsuspendForSuspendingPackage(packageName, userId); + } } } else { succeeded = false; diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index f8bf9c45e8b1a..5c6338dd0a16f 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -734,10 +734,10 @@ public final class Settings { true /*notLaunched*/, false /*hidden*/, false /*suspended*/, - null, /*suspendingPackage*/ - null, /*dialogMessage*/ - null, /*suspendedAppExtras*/ - null, /*suspendedLauncherExtras*/ + null /*suspendingPackage*/, + null /*dialogMessage*/, + null /*suspendedAppExtras*/, + null /*suspendedLauncherExtras*/, instantApp, virtualPreload, null /*lastDisableAppCaller*/, @@ -1634,10 +1634,10 @@ public final class Settings { false /*notLaunched*/, false /*hidden*/, false /*suspended*/, - null, /*suspendingPackage*/ - null, /*dialogMessage*/ - null, /*suspendedAppExtras*/ - null, /*suspendedLauncherExtras*/ + null /*suspendingPackage*/, + null /*dialogMessage*/, + null /*suspendedAppExtras*/, + null /*suspendedLauncherExtras*/, false /*instantApp*/, false /*virtualPreload*/, null /*lastDisableAppCaller*/, diff --git a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java index f6592257b636e..d3526f7613015 100644 --- a/services/core/java/com/android/server/pm/permission/PermissionManagerService.java +++ b/services/core/java/com/android/server/pm/permission/PermissionManagerService.java @@ -1953,7 +1953,7 @@ public class PermissionManagerService { UserManager.DISALLOW_DEBUGGING_FEATURES, callingUid, userId); } if (userId == UserHandle.getUserId(callingUid)) return; - if (callingUid != Process.SYSTEM_UID && callingUid != 0) { + if (callingUid != Process.SYSTEM_UID && callingUid != Process.ROOT_UID) { if (requireFullPermission) { mContext.enforceCallingOrSelfPermission( android.Manifest.permission.INTERACT_ACROSS_USERS_FULL, message);