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

Commit 82278d78 authored by JW Wang's avatar JW Wang
Browse files

Rewrite some methods (3/n)

1. Now #isInstallerDeviceOwnerOrAffiliatedProfileOwner should be called
   outside the lock because it accesses external entities.
2. Now #needToAskForPermissions should be called outside the lock
   for the same reason above.
3. #sendOnUserActionRequired, #sendOnPackageInstalled and
   #sendPendingStreaming are now called outside the lock.
4. Rewrite #installNonStagedLocked so mPm.installStage() is called
   outside the lock.
5. Split #makeSessionActive into 2 parts so permission checks
   can be done outside the lock.

Note the original code is flawed in that
session.makeSessionActiveLocked() [1] is called without session.mLock
held. This is also fixed by this CL.

[1]
https://source.corp.google.com/android/frameworks/base/services/core/java/com/android/server/pm/PackageInstallerSession.java;rcl=d07f2fc343d37100303696fcfeb603232b5087ce;l=1755

Bug: 161199857
Test: atest StagedInstallTest AtomicInstallTest
Change-Id: I1f4a28259f185d8792bac5a9c8a974df53e7d3fb
parent a39f84ee
Loading
Loading
Loading
Loading
+82 −52
Original line number Diff line number Diff line
@@ -459,12 +459,8 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                    final int returnCode = args.argi1;
                    args.recycle();

                    final boolean showNotification;
                    synchronized (mLock) {
                        showNotification = isInstallerDeviceOwnerOrAffiliatedProfileOwnerLocked();
                    }
                    sendOnPackageInstalled(mContext, statusReceiver, sessionId,
                            showNotification, userId,
                            isInstallerDeviceOwnerOrAffiliatedProfileOwner(), userId,
                            packageName, returnCode, message, extras);

                    break;
@@ -494,8 +490,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
    /**
     * @return {@code true} iff the installing is app an device owner or affiliated profile owner.
     */
    @GuardedBy("mLock")
    private boolean isInstallerDeviceOwnerOrAffiliatedProfileOwnerLocked() {
    private boolean isInstallerDeviceOwnerOrAffiliatedProfileOwner() {
        assertNotLocked("isInstallerDeviceOwnerOrAffiliatedProfileOwner");
        // It is safe to access mInstallerUid and mInstallSource without lock
        // because they are immutable after sealing.
        assertSealed("isInstallerDeviceOwnerOrAffiliatedProfileOwner");
        if (userId != UserHandle.getUserId(mInstallerUid)) {
            return false;
        }
@@ -513,12 +512,17 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     *
     * @return {@code true} iff we need to ask to confirm the permissions?
     */
    @GuardedBy("mLock")
    private boolean needToAskForPermissionsLocked() {
    private boolean needToAskForPermissions() {
        final String packageName;
        synchronized (mLock) {
            if (mPermissionsManuallyAccepted) {
                return false;
            }
            packageName = mPackageName;
        }

        // It is safe to access mInstallerUid and mInstallSource without lock
        // because they are immutable after sealing.
        final boolean isInstallPermissionGranted =
                (mPm.checkUidPermission(android.Manifest.permission.INSTALL_PACKAGES,
                        mInstallerUid) == PackageManager.PERMISSION_GRANTED);
@@ -528,7 +532,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        final boolean isUpdatePermissionGranted =
                (mPm.checkUidPermission(android.Manifest.permission.INSTALL_PACKAGE_UPDATES,
                        mInstallerUid) == PackageManager.PERMISSION_GRANTED);
        final int targetPackageUid = mPm.getPackageUid(mPackageName, 0, userId);
        final int targetPackageUid = mPm.getPackageUid(packageName, 0, userId);
        final boolean isPermissionGranted = isInstallPermissionGranted
                || (isUpdatePermissionGranted && targetPackageUid != -1)
                || (isSelfUpdatePermissionGranted && targetPackageUid == mInstallerUid);
@@ -540,7 +544,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        // Device owners and affiliated profile owners  are allowed to silently install packages, so
        // the permission check is waived if the installer is the device owner.
        return forcePermissionPrompt || !(isPermissionGranted || isInstallerRoot
                || isInstallerSystem || isInstallerDeviceOwnerOrAffiliatedProfileOwnerLocked());
                || isInstallerSystem || isInstallerDeviceOwnerOrAffiliatedProfileOwner());
    }

    public PackageInstallerSession(PackageInstallerService.InternalCallback callback,
@@ -740,6 +744,18 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        }
    }

    private void assertNotLocked(String cookie) {
        if (Thread.holdsLock(mLock)) {
            throw new IllegalStateException(cookie + " is holding mLock");
        }
    }

    private void assertSealed(String cookie) {
        if (!isSealed()) {
            throw new IllegalStateException(cookie + " before sealing");
        }
    }

    @GuardedBy("mLock")
    private void assertPreparedAndNotSealedLocked(String cookie) {
        assertPreparedAndNotCommittedOrDestroyedLocked(cookie);
@@ -1693,11 +1709,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
    }

    private void handleInstall() {
        final boolean needsLogging;
        synchronized (mLock) {
            needsLogging = isInstallerDeviceOwnerOrAffiliatedProfileOwnerLocked();
        }
        if (needsLogging) {
        if (isInstallerDeviceOwnerOrAffiliatedProfileOwner()) {
            DevicePolicyEventLogger
                    .createEvent(DevicePolicyEnums.INSTALL_PACKAGE)
                    .setAdmin(mInstallSource.installerPackageName)
@@ -1724,9 +1736,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        List<PackageInstallerSession> childSessions = getChildSessionsNotLocked();

        try {
            synchronized (mLock) {
                installNonStagedLocked(childSessions);
            }
            installNonStaged(childSessions);
        } catch (PackageManagerException e) {
            final String completeMsg = ExceptionUtils.getCompleteMessage(e);
            Slog.e(TAG, "Commit of session " + sessionId + " failed: " + completeMsg);
@@ -1735,11 +1745,10 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
        }
    }

    @GuardedBy("mLock")
    private void installNonStagedLocked(List<PackageInstallerSession> childSessions)
    private void installNonStaged(List<PackageInstallerSession> childSessions)
            throws PackageManagerException {
        final PackageManagerService.ActiveInstallSession installingSession =
                makeSessionActiveLocked();
                makeSessionActive();
        if (installingSession == null) {
            return;
        }
@@ -1752,7 +1761,7 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                final PackageInstallerSession session = childSessions.get(i);
                try {
                    final PackageManagerService.ActiveInstallSession installingChildSession =
                            session.makeSessionActiveLocked();
                            session.makeSessionActive();
                    if (installingChildSession != null) {
                        installingChildSessions.add(installingChildSession);
                    }
@@ -1762,8 +1771,12 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
                }
            }
            if (!success) {
                sendOnPackageInstalled(mContext, mRemoteStatusReceiver, sessionId,
                        isInstallerDeviceOwnerOrAffiliatedProfileOwnerLocked(), userId, null,
                final IntentSender statusReceiver;
                synchronized (mLock) {
                    statusReceiver = mRemoteStatusReceiver;
                }
                sendOnPackageInstalled(mContext, statusReceiver, sessionId,
                        isInstallerDeviceOwnerOrAffiliatedProfileOwner(), userId, null,
                        failure.error, failure.getLocalizedMessage(), null);
                return;
            }
@@ -1778,26 +1791,26 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
     * {@link PackageManagerService.ActiveInstallSession} representing this new staged state or null
     * in case permissions need to be requested before install can proceed.
     */
    @GuardedBy("mLock")
    private PackageManagerService.ActiveInstallSession makeSessionActiveLocked()
    private PackageManagerService.ActiveInstallSession makeSessionActive()
            throws PackageManagerException {
        assertNotLocked("makeSessionActive");

        synchronized (mLock) {
            if (mRelinquished) {
                throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR,
                        "Session relinquished");
            }
            if (mDestroyed) {
            throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, "Session destroyed");
                throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR,
                        "Session destroyed");
            }
            if (!mSealed) {
            throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR, "Session not sealed");
                throw new PackageManagerException(INSTALL_FAILED_INTERNAL_ERROR,
                        "Session not sealed");
            }
        }

        if (!params.isMultiPackage) {
            Objects.requireNonNull(mPackageName);
            Objects.requireNonNull(mSigningDetails);
            Objects.requireNonNull(mResolvedBaseFile);

            if (needToAskForPermissionsLocked()) {
        if (!params.isMultiPackage && needToAskForPermissions()) {
            // User needs to confirm installation;
            // give installer an intent they can use to involve
            // user.
@@ -1805,7 +1818,11 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
            intent.setPackage(mPm.getPackageInstallerPackageName());
            intent.putExtra(PackageInstaller.EXTRA_SESSION_ID, sessionId);

                sendOnUserActionRequired(mContext, mRemoteStatusReceiver, sessionId, intent);
            final IntentSender statusReceiver;
            synchronized (mLock) {
                statusReceiver = mRemoteStatusReceiver;
            }
            sendOnUserActionRequired(mContext, statusReceiver, sessionId, intent);

            // Commit was keeping session marked as active until now; release
            // that extra refcount so session appears idle.
@@ -1813,6 +1830,19 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub {
            return null;
        }

        synchronized (mLock) {
            return makeSessionActiveLocked();
        }
    }

    @GuardedBy("mLock")
    private PackageManagerService.ActiveInstallSession makeSessionActiveLocked()
            throws PackageManagerException {
        if (!params.isMultiPackage) {
            Objects.requireNonNull(mPackageName);
            Objects.requireNonNull(mSigningDetails);
            Objects.requireNonNull(mResolvedBaseFile);

            // Inherit any packages and native libraries from existing install that
            // haven't been overridden.
            if (params.mode == SessionParams.MODE_INHERIT_EXISTING) {