Support system app update leaving shared UID then getting uninstalled

It is possible for a system app to leave shared user ID by an update. We
need to keep track of this in case the upgrade is uninstalled. In this
scenario, the shadowed system package will be reinstalled using the
original shared UID. This has the side effect of the app ID used by the
shared user group being held forever even if no apps is actively using
it, because only the app ID is stored in packages.xml, not the
sharedUserId name, and we have to use the app ID to link the disabled
PackageSetting with the corresponding SharedUserSetting.

Test: tracking with b/202993696
Bug: 179284822
Fixes: 201509122
Change-Id: If48aeb7d63daf1ef8bdf06a9d2dd34c1de8603c8

4 files changed, 99 insertions, 89 deletions

diff --git a/services/core/java/com/android/server/pm/DeletePackageHelper.java b/services/core/java/com/android/server/pm/DeletePackageHelper.java
index 37879d7186a7..7dd02cb4ec2b 100644
--- a/services/core/java/com/android/server/pm/DeletePackageHelper.java
+++ b/services/core/java/com/android/server/pm/DeletePackageHelper.java
@@ -458,10 +458,8 @@ final class DeletePackageHelper {
         mAppDataHelper.destroyAppProfilesLIF(pkg);
 
         final SharedUserSetting sus = ps.getSharedUser();
-        List<AndroidPackage> sharedUserPkgs = sus != null ? sus.getPackages() : null;
-        if (sharedUserPkgs == null) {
-            sharedUserPkgs = Collections.emptyList();
-        }
+        final List<AndroidPackage> sharedUserPkgs =
+                sus != null ? sus.getPackages() : Collections.emptyList();
         final int[] userIds = (userId == UserHandle.USER_ALL) ? mUserManagerInternal.getUserIds()
                 : new int[] {userId};
         for (int nextUserId : userIds) {
diff --git a/services/core/java/com/android/server/pm/RemovePackageHelper.java b/services/core/java/com/android/server/pm/RemovePackageHelper.java
index 7596cdf123c6..bcf2f921d55b 100644
--- a/services/core/java/com/android/server/pm/RemovePackageHelper.java
+++ b/services/core/java/com/android/server/pm/RemovePackageHelper.java
@@ -220,9 +220,8 @@ final class RemovePackageHelper {
             outInfo.mInstallerPackageName = deletedPs.getInstallSource().installerPackageName;
             outInfo.mIsStaticSharedLib = deletedPkg != null
                     && deletedPkg.getStaticSharedLibName() != null;
-            outInfo.populateUsers(
-                    deletedPs == null ? null : deletedPs.queryInstalledUsers(
-                            mUserManagerInternal.getUserIds(), true), deletedPs);
+            outInfo.populateUsers(deletedPs.queryInstalledUsers(
+                    mUserManagerInternal.getUserIds(), true), deletedPs);
         }
 
         removePackageLI(deletedPs.getPackageName(), (flags & PackageManager.DELETE_CHATTY) != 0);
@@ -249,58 +248,53 @@ final class RemovePackageHelper {
 
         // writer
         boolean installedStateChanged = false;
-        if (deletedPs != null) {
-            if ((flags & PackageManager.DELETE_KEEP_DATA) == 0) {
-                final SparseBooleanArray changedUsers = new SparseBooleanArray();
-                synchronized (mPm.mLock) {
-                    mPm.mDomainVerificationManager.clearPackage(deletedPs.getPackageName());
-                    mPm.mSettings.getKeySetManagerService().removeAppKeySetDataLPw(packageName);
-                    mPm.mAppsFilter.removePackage(mPm.getPackageSetting(packageName),
-                            false /* isReplace */);
-                    removedAppId = mPm.mSettings.removePackageLPw(packageName);
-                    if (outInfo != null) {
-                        outInfo.mRemovedAppId = removedAppId;
-                    }
-                    if (!mPm.mSettings.isDisabledSystemPackageLPr(packageName)) {
-                        // If we don't have a disabled system package to reinstall, the package is
-                        // really gone and its permission state should be removed.
-                        final SharedUserSetting sus = deletedPs.getSharedUser();
-                        List<AndroidPackage> sharedUserPkgs = sus != null ? sus.getPackages()
-                                : null;
-                        if (sharedUserPkgs == null) {
-                            sharedUserPkgs = Collections.emptyList();
-                        }
-                        mPermissionManager.onPackageUninstalled(packageName, deletedPs.getAppId(),
-                                deletedPs.getPkg(), sharedUserPkgs, UserHandle.USER_ALL);
-                    }
-                    mPm.clearPackagePreferredActivitiesLPw(
-                            deletedPs.getPackageName(), changedUsers, UserHandle.USER_ALL);
-
-                    mPm.mSettings.removeRenamedPackageLPw(deletedPs.getRealName());
+        if ((flags & PackageManager.DELETE_KEEP_DATA) == 0) {
+            final SparseBooleanArray changedUsers = new SparseBooleanArray();
+            synchronized (mPm.mLock) {
+                mPm.mDomainVerificationManager.clearPackage(deletedPs.getPackageName());
+                mPm.mSettings.getKeySetManagerService().removeAppKeySetDataLPw(packageName);
+                mPm.mAppsFilter.removePackage(mPm.getPackageSetting(packageName),
+                        false /* isReplace */);
+                removedAppId = mPm.mSettings.removePackageLPw(packageName);
+                if (outInfo != null) {
+                    outInfo.mRemovedAppId = removedAppId;
                 }
-                if (changedUsers.size() > 0) {
-                    mPm.updateDefaultHomeNotLocked(changedUsers);
-                    mPm.postPreferredActivityChangedBroadcast(UserHandle.USER_ALL);
+                if (!mPm.mSettings.isDisabledSystemPackageLPr(packageName)) {
+                    // If we don't have a disabled system package to reinstall, the package is
+                    // really gone and its permission state should be removed.
+                    final SharedUserSetting sus = deletedPs.getSharedUser();
+                    List<AndroidPackage> sharedUserPkgs =
+                            sus != null ? sus.getPackages() : Collections.emptyList();
+                    mPermissionManager.onPackageUninstalled(packageName, deletedPs.getAppId(),
+                            deletedPs.getPkg(), sharedUserPkgs, UserHandle.USER_ALL);
                 }
+                mPm.clearPackagePreferredActivitiesLPw(
+                        deletedPs.getPackageName(), changedUsers, UserHandle.USER_ALL);
+
+                mPm.mSettings.removeRenamedPackageLPw(deletedPs.getRealName());
+            }
+            if (changedUsers.size() > 0) {
+                mPm.updateDefaultHomeNotLocked(changedUsers);
+                mPm.postPreferredActivityChangedBroadcast(UserHandle.USER_ALL);
             }
-            // make sure to preserve per-user disabled state if this removal was just
-            // a downgrade of a system app to the factory package
-            if (outInfo != null && outInfo.mOrigUsers != null) {
+        }
+        // make sure to preserve per-user disabled state if this removal was just
+        // a downgrade of a system app to the factory package
+        if (outInfo != null && outInfo.mOrigUsers != null) {
+            if (DEBUG_REMOVE) {
+                Slog.d(TAG, "Propagating install state across downgrade");
+            }
+            for (int userId : allUserHandles) {
+                final boolean installed = ArrayUtils.contains(outInfo.mOrigUsers, userId);
                 if (DEBUG_REMOVE) {
-                    Slog.d(TAG, "Propagating install state across downgrade");
+                    Slog.d(TAG, "    user " + userId + " => " + installed);
                 }
-                for (int userId : allUserHandles) {
-                    final boolean installed = ArrayUtils.contains(outInfo.mOrigUsers, userId);
-                    if (DEBUG_REMOVE) {
-                        Slog.d(TAG, "    user " + userId + " => " + installed);
-                    }
-                    if (installed != deletedPs.getInstalled(userId)) {
-                        installedStateChanged = true;
-                    }
-                    deletedPs.setInstalled(installed, userId);
-                    if (installed) {
-                        deletedPs.setUninstallReason(UNINSTALL_REASON_UNKNOWN, userId);
-                    }
+                if (installed != deletedPs.getInstalled(userId)) {
+                    installedStateChanged = true;
+                }
+                deletedPs.setInstalled(installed, userId);
+                if (installed) {
+                    deletedPs.setUninstallReason(UNINSTALL_REASON_UNKNOWN, userId);
                 }
             }
         }
diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java
index 85adaa0c845b..0902e4a3112c 100644
--- a/services/core/java/com/android/server/pm/Settings.java
+++ b/services/core/java/com/android/server/pm/Settings.java
@@ -802,7 +802,9 @@ public final class Settings implements Watchable, Snappable {
                 disabled = p;
             }
             mDisabledSysPackages.put(name, disabled);
-
+            if (disabled.getSharedUser() != null) {
+                disabled.getSharedUser().mDisabledPackages.add(disabled);
+            }
             return true;
         }
         return false;
@@ -814,6 +816,9 @@ public final class Settings implements Watchable, Snappable {
             Log.w(PackageManagerService.TAG, "Package " + name + " is not disabled");
             return null;
         }
+        if (p.getSharedUser() != null) {
+            p.getSharedUser().mDisabledPackages.remove(p);
+        }
         p.getPkgState().setUpdatedSystemApp(false);
         PackageSetting ret = addPackageLPw(name, p.getRealName(), p.getPath(),
                 p.getLegacyNativeLibraryPath(), p.getPrimaryCpuAbi(),
@@ -833,7 +838,11 @@ public final class Settings implements Watchable, Snappable {
     }
 
     void removeDisabledSystemPackageLPw(String name) {
-        mDisabledSysPackages.remove(name);
+        final PackageSetting p = mDisabledSysPackages.remove(name);
+        if (p != null && p.getSharedUser() != null) {
+            p.getSharedUser().mDisabledPackages.remove(p);
+            checkAndPruneSharedUserLPw(p.getSharedUser(), false);
+        }
     }
 
     PackageSetting addPackageLPw(String name, String realName, File codePath,
@@ -883,27 +892,24 @@ public final class Settings implements Watchable, Snappable {
     }
 
     void pruneSharedUsersLPw() {
-        ArrayList<String> removeStage = new ArrayList<String>();
-        for (Map.Entry<String,SharedUserSetting> entry : mSharedUsers.entrySet()) {
+        List<String> removeKeys = new ArrayList<>();
+        List<SharedUserSetting> removeValues = new ArrayList<>();
+        for (Map.Entry<String, SharedUserSetting> entry : mSharedUsers.entrySet()) {
             final SharedUserSetting sus = entry.getValue();
             if (sus == null) {
-                removeStage.add(entry.getKey());
+                removeKeys.add(entry.getKey());
                 continue;
             }
             // remove packages that are no longer installed
-            for (Iterator<PackageSetting> iter = sus.packages.iterator(); iter.hasNext();) {
-                PackageSetting ps = iter.next();
-                if (mPackages.get(ps.getPackageName()) == null) {
-                    iter.remove();
-                }
-            }
-            if (sus.packages.size() == 0) {
-                removeStage.add(entry.getKey());
+            sus.packages.removeIf(ps -> mPackages.get(ps.getPackageName()) == null);
+            sus.mDisabledPackages.removeIf(
+                    ps -> mDisabledSysPackages.get(ps.getPackageName()) == null);
+            if (sus.packages.isEmpty() && sus.mDisabledPackages.isEmpty()) {
+                removeValues.add(sus);
             }
         }
-        for (int i = 0; i < removeStage.size(); i++) {
-            mSharedUsers.remove(removeStage.get(i));
-        }
+        removeKeys.forEach(mSharedUsers::remove);
+        removeValues.forEach(sus -> checkAndPruneSharedUserLPw(sus, true));
     }
 
     /**
@@ -1233,18 +1239,20 @@ public final class Settings implements Watchable, Snappable {
         }
     }
 
+    private void checkAndPruneSharedUserLPw(SharedUserSetting s, boolean skipCheck) {
+        if (skipCheck || (s.packages.isEmpty() && s.mDisabledPackages.isEmpty())) {
+            mSharedUsers.remove(s.name);
+            removeAppIdLPw(s.userId);
+        }
+    }
+
     int removePackageLPw(String name) {
-        final PackageSetting p = mPackages.get(name);
+        final PackageSetting p = mPackages.remove(name);
         if (p != null) {
-            mPackages.remove(name);
             removeInstallerPackageStatus(name);
             if (p.getSharedUser() != null) {
                 p.getSharedUser().removePackage(p);
-                if (p.getSharedUser().packages.size() == 0) {
-                    mSharedUsers.remove(p.getSharedUser().name);
-                    removeAppIdLPw(p.getSharedUser().userId);
-                    return p.getSharedUser().userId;
-                }
+                checkAndPruneSharedUserLPw(p.getSharedUser(), false);
             } else {
                 removeAppIdLPw(p.getAppId());
                 return p.getAppId();
@@ -3052,17 +3060,16 @@ public final class Settings implements Watchable, Snappable {
          * Make sure all the updated system packages have their shared users
          * associated with them.
          */
-        final Iterator<PackageSetting> disabledIt = mDisabledSysPackages.values().iterator();
-        while (disabledIt.hasNext()) {
-            final PackageSetting disabledPs = disabledIt.next();
+        for (PackageSetting disabledPs : mDisabledSysPackages.values()) {
             final Object id = getSettingLPr(disabledPs.getAppId());
-            if (id != null && id instanceof SharedUserSetting) {
+            if (id instanceof SharedUserSetting) {
                 disabledPs.setSharedUser((SharedUserSetting) id);
+                disabledPs.getSharedUser().mDisabledPackages.add(disabledPs);
             }
         }
 
-        mReadMessages.append("Read completed successfully: " + mPackages.size() + " packages, "
-                + mSharedUsers.size() + " shared uids\n");
+        mReadMessages.append("Read completed successfully: ").append(mPackages.size())
+                .append(" packages, ").append(mSharedUsers.size()).append(" shared uids\n");
 
         writeKernelMappingLPr();
 
diff --git a/services/core/java/com/android/server/pm/SharedUserSetting.java b/services/core/java/com/android/server/pm/SharedUserSetting.java
index 15df2496a831..3055747045bd 100644
--- a/services/core/java/com/android/server/pm/SharedUserSetting.java
+++ b/services/core/java/com/android/server/pm/SharedUserSetting.java
@@ -16,7 +16,7 @@
 
 package com.android.server.pm;
 
-import android.annotation.Nullable;
+import android.annotation.NonNull;
 import android.content.pm.ApplicationInfo;
 import android.content.pm.parsing.component.ParsedProcess;
 import android.service.pm.PackageServiceDumpProto;
@@ -31,6 +31,7 @@ import com.android.server.utils.SnapshotCache;
 import libcore.util.EmptyArray;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -52,6 +53,11 @@ public final class SharedUserSetting extends SettingBase {
 
     final ArraySet<PackageSetting> packages;
 
+    // It is possible for a system app to leave shared user ID by an update.
+    // We need to keep track of the shadowed PackageSettings so that it is possible to uninstall
+    // the update and revert the system app back into the original shared user ID.
+    final ArraySet<PackageSetting> mDisabledPackages;
+
     final PackageSignatures signatures = new PackageSignatures();
     Boolean signaturesChanged;
 
@@ -77,6 +83,7 @@ public final class SharedUserSetting extends SettingBase {
         name = _name;
         seInfoTargetSdkVersion = android.os.Build.VERSION_CODES.CUR_DEVELOPMENT;
         packages = new ArraySet<>();
+        mDisabledPackages = new ArraySet<>();
         processes = new ArrayMap<>();
         mSnapshot = makeCache();
     }
@@ -87,13 +94,14 @@ public final class SharedUserSetting extends SettingBase {
         name = orig.name;
         uidFlags = orig.uidFlags;
         uidPrivateFlags = orig.uidPrivateFlags;
-        packages = new ArraySet(orig.packages);
+        packages = new ArraySet<>(orig.packages);
+        mDisabledPackages = new ArraySet<>(orig.mDisabledPackages);
         // A SigningDetails seems to consist solely of final attributes, so
         // it is safe to copy the reference.
         signatures.mSigningDetails = orig.signatures.mSigningDetails;
         signaturesChanged = orig.signaturesChanged;
-        processes = new ArrayMap(orig.processes);
-        mSnapshot = new SnapshotCache.Sealed();
+        processes = new ArrayMap<>(orig.processes);
+        mSnapshot = new SnapshotCache.Sealed<>();
     }
 
     /**
@@ -174,9 +182,12 @@ public final class SharedUserSetting extends SettingBase {
         }
     }
 
-    public @Nullable List<AndroidPackage> getPackages() {
+    /**
+     * @return the list of packages that uses this shared UID
+     */
+    public @NonNull List<AndroidPackage> getPackages() {
         if (packages == null || packages.size() == 0) {
-            return null;
+            return Collections.emptyList();
         }
         final ArrayList<AndroidPackage> pkgList = new ArrayList<>(packages.size());
         for (PackageSetting ps : packages) {