Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ea789fd6 authored by Hai Zhang's avatar Hai Zhang
Browse files

Refactor and rename onPackageStateRemoved() to onPackageUninstalled().

Under what conditions to notify the permission subsystem that a
package is considered uninstalled can be tricky/confusing, because
there are so many different code paths and flavors for deleting a
package, deleting a package can mean different things, and it's
critical to get it right for permissions due to security concerns. If
a package is being deleted as a upgrade step, we should not consider
it an uninstallation (but a later package installation with replaced
== true), and handle the <uses-permission> change later in that
callback. This is also true for uninstalling an updated system package
and reinstalling the factory version later, but that's a different
code path.

For system packages, we normally can't just completely remove their
permission state upon uninstallation, because they can be marked as
installed later, and permission pregrants needs to stick for them. So
in this case we just clear the user modifiable permission state
instead of dropping the whole state. However, we cannot assume that we
should never drop the permission state for a system package, because
there are package uninstallations during the package reconciliation
phase of system boot that we do need to remove the permission state
for invalid (e.g. missing) system packages.

DELETE_KEEP_DATA as a flag for deleting a package actually means two
different things when marking package as uninstalled v.s. removing
package for all users. When marking a package as uninstalled, this
flag only affects whether the data directory is deleted (as what is
described in its javadoc), and all other states (including permission
state) is always deleted because it's an uninstallation for that
user. However when removing a package for all users, specifying this
flag actually means keeping all package states and only remove the
APK, which is also used when an external SD card is ejected (or a
package is upgraded, or uninstalling an updated system package which
has a lower version than its factory version due to OTA). In other
words, uninstalling a package for all users with DELETE_KEEP_DATA
actually keeps the package installed and all its states (including
PackageSetting) intact, so it shouldn't be considred uninstalled for
permission; whereas uninstalling for a subset of users with
DELETE_KEEP_DATA marks the package uninstalled and removes most
package state for those users except for the data directory, so it
should be considered uninstalled for permission.

Another subtle aspect of DELETE_KEEP_DATA is that it is always set
when deleting an updated system app whose version is lower than its
factory version due to OTA, but never set when the factory version is
older, i.e. a downgrade, which is more often the case. In this case we
are deleting most of the package states, however we do need to keep
the permission state mostly intact, as to user it's only a replacement
so user preference should be carried over, and the downgraded app
cannot be missing pregranted permissions anyway. So the current
implementation is that we need to clear the no longer requested
permission states if the app has a shared user ID, or leave it as-is
if it doesn't as permissions not requested won't be reported as
granted.

According to the discussion above, we are no longer calling the
onPackageUninstalled() callback when a updated system package is being
uninstalled for later installation of its factory version, because it
should be considered as a package replacement instead of an
uninstallation that should remove/reset its permission state. This is
covered by the call to updatePermissions() inside
installPackageFromSystemLIF() called by deleteSystemPackageLIF(),
which is also moved a bit later to allow correct installed state to be
set first. The method updatePermissions() should be later refactored
into onPackageInstalled().

The call to updatePermissions() in the old onPackageStateRemoved() is
actually necessary because it removes permission definitions
according. On the contrary, removeAllPermissions() called in
onPackageRemoved() only removes the PermissionInfo object in
Permission, instead of the Permission itself, because it may be called
upon SD card ejection.

Bug: 158736025
Test: presubmit
Change-Id: I06321ba21ee9e13a9bfbb1c990bbd50a68503b88
parent 6b086884
Loading
Loading
Loading
Loading
+30 −13
Original line number Diff line number Diff line
@@ -19497,13 +19497,20 @@ public class PackageManagerService extends IPackageManager.Stub
                    if (outInfo != null) {
                        outInfo.removedAppId = removedAppId;
                    }
                    if (!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;
                        List<AndroidPackage> sharedUserPkgs = sus != null ? sus.getPackages()
                                : null;
                        if (sharedUserPkgs == null) {
                            sharedUserPkgs = Collections.emptyList();
                        }
                    mPermissionManager.onPackageStateRemoved(packageName, deletedPs.appId,
                            deletedPs.pkg, sharedUserPkgs);
                        for (final int userId : allUserHandles) {
                            mPermissionManager.onPackageUninstalled(packageName, deletedPs.appId,
                                    deletedPs.pkg, sharedUserPkgs, userId);
                        }
                    }
                    clearPackagePreferredActivitiesLPw(
                            deletedPs.name, changedUsers, UserHandle.USER_ALL);
                }
@@ -19692,10 +19699,6 @@ public class PackageManagerService extends IPackageManager.Stub
        synchronized (mLock) {
            PackageSetting ps = mSettings.mPackages.get(pkg.getPackageName());
            // The update permissions method below will take care of removing obsolete permissions
            // and granting install permissions.
            mPermissionManager.updatePermissions(pkg.getPackageName(), pkg);
            final boolean applyUserRestrictions = origUserHandles != null;
            if (applyUserRestrictions) {
                boolean installedStateChanged = false;
@@ -19714,8 +19717,6 @@ public class PackageManagerService extends IPackageManager.Stub
                    if (installed) {
                        ps.setUninstallReason(UNINSTALL_REASON_UNKNOWN, userId);
                    }
                    mSettings.writeRuntimePermissionsForUserLPr(userId, false);
                }
                // Regardless of writeSettings we need to ensure that this restriction
                // state propagation is persisted
@@ -19724,6 +19725,16 @@ public class PackageManagerService extends IPackageManager.Stub
                    mSettings.writeKernelMappingLPr(ps);
                }
            }
            // The update permissions method below will take care of removing obsolete permissions
            // and granting install permissions.
            mPermissionManager.updatePermissions(pkg.getPackageName(), pkg);
            if (applyUserRestrictions) {
                for (int userId : allUserHandles) {
                    mSettings.writeRuntimePermissionsForUserLPr(userId, false);
                }
            }
            // can downgrade to reader here
            if (writeSettings) {
                writeSettingsLPrTEMP();
@@ -20022,6 +20033,11 @@ public class PackageManagerService extends IPackageManager.Stub
        destroyAppProfilesLIF(pkg);
        final SharedUserSetting sus = ps.getSharedUser();
        List<AndroidPackage> sharedUserPkgs = sus != null ? sus.getPackages() : null;
        if (sharedUserPkgs == null) {
            sharedUserPkgs = Collections.emptyList();
        }
        final int[] userIds = (userId == UserHandle.USER_ALL) ? mUserManager.getUserIds()
                : new int[] {userId};
        for (int nextUserId : userIds) {
@@ -20036,7 +20052,8 @@ public class PackageManagerService extends IPackageManager.Stub
            clearDefaultBrowserIfNeededForUser(ps.name, nextUserId);
            removeKeystoreDataIfNeeded(mInjector.getUserManagerInternal(), nextUserId, ps.appId);
            clearPackagePreferredActivities(ps.name, nextUserId);
            mPermissionManager.resetRuntimePermissions(pkg, nextUserId);
            mPermissionManager.onPackageUninstalled(ps.name, ps.appId, pkg, sharedUserPkgs,
                    nextUserId);
        }
        if (outInfo != null) {
+33 −22
Original line number Diff line number Diff line
@@ -4664,13 +4664,13 @@ public class PermissionManagerService extends IPermissionManager.Stub {
        return userState.getUidState(appId);
    }

    private void removeAppIdState(@AppIdInt int appId) {
    private void removeUidState(@AppIdInt int appId, @UserIdInt int userId) {
        synchronized (mLock) {
            final int[] userIds = mState.getUserIds();
            for (final int userId : userIds) {
            final UserPermissionState userState = mState.getUserState(userId);
                userState.removeUidState(appId);
            if (userState == null) {
                return;
            }
            userState.removeUidState(appId);
        }
    }

@@ -4929,25 +4929,34 @@ public class PermissionManagerService extends IPermissionManager.Stub {
        removeAllPermissionsInternal(pkg);
    }

    private void onPackageStateRemovedInternal(@NonNull String packageName, int appId,
            @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs) {
        if (sharedUserPkgs.isEmpty()
                && mPackageManagerInt.getDisabledSystemPackage(packageName) == null) {
            removeAppIdState(appId);
    private void onPackageUninstalledInternal(@NonNull String packageName, int appId,
            @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs,
            @UserIdInt int userId) {
        // TODO: Move these checks to check PackageState to be more reliable.
        // System packages should always have an available APK.
        if (pkg != null && pkg.isSystem()
                // We may be fully removing invalid system packages during boot, and in that case we
                // do want to remove their permission state. So make sure that the package is only
                // being marked as uninstalled instead of fully removed.
                && mPackageManagerInt.getPackage(packageName) != null) {
            // If we are only marking a system package as uninstalled, we need to keep its
            // pregranted permission state so that it still works once it gets reinstalled, thus
            // only reset the user modifications to its permission state.
            resetRuntimePermissionsInternal(pkg, userId);
            return;
        }
        updatePermissions(packageName, null, mDefaultPermissionCallback);
        if (!sharedUserPkgs.isEmpty()) {
        if (sharedUserPkgs.isEmpty()) {
            removeUidState(appId, userId);
        } else {
            // Remove permissions associated with package. Since runtime
            // permissions are per user we have to kill the removed package
            // or packages running under the shared user of the removed
            // package if revoking the permissions requested only by the removed
            // package is successful and this causes a change in gids.
            boolean shouldKill = false;
            for (int userId : UserManagerService.getInstance().getUserIds()) {
            final int userIdToKill = revokeSharedUserPermissionsForDeletedPackageInternal(pkg,
                    sharedUserPkgs, userId);
                shouldKill |= userIdToKill != UserHandle.USER_NULL;
            }
            final boolean shouldKill = userIdToKill != UserHandle.USER_NULL;
            // If gids changed, kill all affected packages.
            if (shouldKill) {
                mHandler.post(() -> {
@@ -5381,11 +5390,13 @@ public class PermissionManagerService extends IPermissionManager.Stub {
        }

        @Override
        public void onPackageStateRemoved(@NonNull String packageName, int appId,
                @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs) {
            Objects.requireNonNull(packageName);
            Objects.requireNonNull(sharedUserPkgs);
            onPackageStateRemovedInternal(packageName, appId, pkg, sharedUserPkgs);
        public void onPackageUninstalled(@NonNull String packageName, int appId,
                @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs,
                @UserIdInt int userId) {
            Objects.requireNonNull(packageName, "packageName");
            Objects.requireNonNull(sharedUserPkgs, "sharedUserPkgs");
            Preconditions.checkArgumentNonNegative(userId, "userId");
            onPackageUninstalledInternal(packageName, appId, pkg, sharedUserPkgs, userId);
        }

        @Override
+13 −6
Original line number Diff line number Diff line
@@ -512,16 +512,23 @@ public abstract class PermissionManagerServiceInternal extends PermissionManager
    public abstract void onPackageRemoved(@NonNull AndroidPackage pkg);

    /**
     * Callback when the state for a package has been removed.
     * Callback when a package has been uninstalled.
     * <p>
     * The package may have been fully removed from the system, or only marked as uninstalled for
     * this user but still instlaled for other users.
     *
     * TODO: Pass PackageState instead.
     *
     * @param packageName the name of the removed package
     * @param appId the app ID of the removed package
     * @param pkg the removed package, or {@code null} if unavailable
     * @param packageName the name of the uninstalled package
     * @param appId the app ID of the uninstalled package
     * @param pkg the uninstalled package, or {@code null} if unavailable
     * @param sharedUserPkgs the packages that are in the same shared user
     * @param userId the user ID the package is uninstalled for
     */
    //@SystemApi(client = SystemApi.Client.SYSTEM_SERVER)
    public abstract void onPackageStateRemoved(@NonNull String packageName, int appId,
            @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs);
    public abstract void onPackageUninstalled(@NonNull String packageName, int appId,
            @Nullable AndroidPackage pkg, @NonNull List<AndroidPackage> sharedUserPkgs,
            @UserIdInt int userId);

    /**
     * Check whether a permission can be propagated to instant app.