From d8597aa9a5bc8def9520bb78bb6e9b368292cbdc Mon Sep 17 00:00:00 2001 From: John Wu Date: Mon, 11 Oct 2021 13:28:26 -0700 Subject: 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 --- .../com/android/server/pm/DeletePackageHelper.java | 6 +- .../com/android/server/pm/RemovePackageHelper.java | 94 ++++++++++------------ .../core/java/com/android/server/pm/Settings.java | 65 ++++++++------- .../com/android/server/pm/SharedUserSetting.java | 23 ++++-- 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 sharedUserPkgs = sus != null ? sus.getPackages() : null; - if (sharedUserPkgs == null) { - sharedUserPkgs = Collections.emptyList(); - } + final List 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 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 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 removeStage = new ArrayList(); - for (Map.Entry entry : mSharedUsers.entrySet()) { + List removeKeys = new ArrayList<>(); + List removeValues = new ArrayList<>(); + for (Map.Entry 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 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 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 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 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 getPackages() { + /** + * @return the list of packages that uses this shared UID + */ + public @NonNull List getPackages() { if (packages == null || packages.size() == 0) { - return null; + return Collections.emptyList(); } final ArrayList pkgList = new ArrayList<>(packages.size()); for (PackageSetting ps : packages) { -- cgit v1.2.3-59-g8ed1b